Skip to content

Fix/master reset after derivation#1735

Merged
merryman merged 2 commits intomainfrom
fix/master-reset-after-derivation
Mar 6, 2025
Merged

Fix/master reset after derivation#1735
merryman merged 2 commits intomainfrom
fix/master-reset-after-derivation

Conversation

@merryman
Copy link
Member

@merryman merryman commented Mar 3, 2025

Fixes #1731.

merryman added 2 commits March 3, 2025 19:20
When estimating the extent properties inside style policies due to breakpoint switches during generation of a build spec from a style policy, we accidentally assigned the policies that contained estimated extents, which lead to weird behavior when triggering different breakpoints.
@merryman merryman requested a review from linusha March 3, 2025 18:23
delete morphInScope._skipMasterReplacement;
return;
if (!morphInScope.master) {
morphInScope.setProperty('master', synthesizedSpec); // how can we carry over overridden props?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to merge regardless, but this comment makes me worry - is there a hidden footgun in this? If so, have you made a note of that somewhere/could you open a ticket, so that we can be sure to tackle this at some point and not forget?

If there is nothing to worry about, maybe just delete the comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we fix the issue mentioned in the comment by not setting the master if there is already one present. That way we avoid the issue of having to worry about carrying over the overridden props.

However what won't properly work now out of the box, is manually swapping out (via aMorph.master = ...) the master of a morph which comes with a set of other components for its submorphs. We don't support this modus of operation for masters since a while now anyways, since we do everything via component definitions and custom master states for swapping out masters.

But I will look and check if there is issues about this before merging.

@merryman merryman merged commit 30cc186 into main Mar 6, 2025
4 checks passed
@merryman merryman deleted the fix/master-reset-after-derivation branch March 6, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Polygons not rendering

2 participants