Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebuild stack card when a child card rebuilds #19861

Merged
merged 1 commit into from Apr 12, 2024

Conversation

karwosts
Copy link
Contributor

@karwosts karwosts commented Feb 22, 2024

Proposed change

I'm unsure if this is the correct fix, but this proposal does fix a bug I have encountered where the masonry layout is not stable in some cases, because cardsize is not calculated for custom cards in stacks the first time the page loads.

This cause the masonry layout to be unstable and cards to move around depending on if it is the first or subsequent time a page is visited.

As a fix, this proposes anytime a card in a stack fires the ll-rebuild event, this has the stack also fire a rebuild event, so that the stack is rebuilt and the size is properly calculated.

Unsure if this is the best performance solution, or if a different event which just retriggered createColumns, but not necessarily recreating the element over again would be preferable.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@bramkragten
Copy link
Member

Why does a rebuild of the stack card trigger the masonry layout to change? I don't think that event reaches the view? 🤔

@bramkragten
Copy link
Member

What I do see, is that we normally recreate the entire view when ll-rebuild is fired in edit mode, but the stack card doesnt seem to do this and always stops the propagation.

But that should only affect edit mode and not normal mode...

@bramkragten
Copy link
Member

bramkragten commented Feb 27, 2024

because cardsize is not calculated for custom cards in stacks the first time the page loads.

This would be a bug in our computeCardSize function btw, as it should wait for the card to be defined.

@karwosts
Copy link
Contributor Author

karwosts commented Feb 27, 2024

What I do see, is that we normally recreate the entire view when ll-rebuild is fired in edit mode, but the stack card doesnt seem to do this and always stops the propagation.

When not in edit mode, ll-rebuild of a top level card (not in a stack) goes to hui-view, and calls rebuildCard.

rebuildCard causes this._cards to be reassigned.

    this._cards = this._cards!.map((curCardEl) =>
      curCardEl === cardElToReplace ? newCardEl : curCardEl
    );

When cards is reassigned, I believe this cause hui-masonry-view to trigger _createColumns

    if (
      changedProperties.has("cards") ||
      (changedProperties.has("lovelace") &&
        oldLovelace &&
        (oldLovelace.config !== this.lovelace!.config ||
          oldLovelace.editMode !== this.lovelace!.editMode))
    ) {
      this._createColumns();
    }

So when a top level card is rebuilt, it triggers masonry to re-layout the columns (and rerun computecardsize)

When a card in a stack is rebuilt, the stack eats the rebuild event and so column layout is not retriggered.

That's why late loading custom cards cardSize looks right when they're placed directly in the root of the view, but not inside a stack.

@bramkragten
Copy link
Member

OK, gotcha... It calculates the height of the error card ofc... thats why it is wrong 🤔

@bramkragten bramkragten merged commit b35c325 into home-assistant:dev Apr 12, 2024
14 checks passed
@karwosts karwosts deleted the rebuild-stacks-on-rebuild branch April 12, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getCardSize isn't called on custom cards in vertical stack on initial load
2 participants