Fix more-info chart rendering #23619
Conversation
|
@karwosts can you try this PR and see if opening a more info dialog with a chart is improved, pls? |
|
This also removed the initial animation I think? So can we remove the |
|
I think we can even remove all the |
|
One thing that doesn't work well anymore now, is resizing. Try to resize the more info window (click the title, or resize your window). I think this also changed the logic for dashboard cards, if it is better or worse I dont know 😄 , but means we cant release this as a patch then. Previously the height would stay the same, now it changes the height if the width changes, but not directly as it first needs a call to
|
|
fixed it to the initial height now |
I think we should move that logic to |
| @@ -84,8 +84,6 @@ export class StatisticsChart extends LitElement { | |||
|
|
|||
| @property() public period?: string; | |||
|
|
|||
| @property({ attribute: false, type: Number }) public height?: number; | |||
There was a problem hiding this comment.
Should we remove the height option completely?
| @@ -69,8 +69,6 @@ export class StateHistoryCharts extends LitElement { | |||
|
|
|||
| @property({ attribute: "fit-y-data", type: Boolean }) public fitYData = false; | |||
|
|
|||
| @property({ attribute: false, type: Number }) public height?: number; | |||
There was a problem hiding this comment.
Should we remove the height option completely? I think we can keep this?
| @@ -54,8 +54,6 @@ export class StateHistoryChartLine extends LitElement { | |||
|
|
|||
| @property({ attribute: "fit-y-data", type: Boolean }) public fitYData = false; | |||
|
|
|||
| @property({ attribute: false, type: Number }) public height?: number; | |||
There was a problem hiding this comment.
Should we remove the height option completely? I think we can keep this?
There was a problem hiding this comment.
It wasn't there before so isn't used. I didn't want to add extra attributes without a use case. We can always add it if we need it.
The more info dialog seems better on this branch, but all my energy cards are undesirably huge now (really big bottom padding), and the loading is sometimes really janky. I've never seen anything like this before.
|
|
Aha, so this is what |
| if (!this.height) { | ||
| // lock the height | ||
| // this removes empty space below the chart | ||
| this.height = chart.height; |
There was a problem hiding this comment.
We should also add back the hysteresis then?
There was a problem hiding this comment.
Not needed anymore as we don't animate it. Only lock it once after the first render
* Fix more-info chart rendering * lint fix * remove animation-container & _chartHeight * don't change height on resize * handle default height in ha-chart-base * fix chart height in energy panel * lint * lint
Proposed change
This removes a resize action after the dialog is open and instead just sets a fixed height to achieve the same size. All the
resizemethods were only used for this so I removed them.I don't know if this will entirely fix the slow render issue since I can't reproduce it but it should at least help.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: