Skip to content

Commit 41bf46d

Browse files
ekwokarobtfm
andauthored
🐛 Fixes PropagateStop and PropagateOver (#21622)
# Objective Fixes #21620 Ensures `PropagateStop` works as documented. At it stands, `PropagateStop<C>` is described as only preventing it's Children from inheriting `C` but in practice that entity itself would not inherit `C`. Makes `PropagateOver` work. It just plain didn't work at all. ## Solution Adjusted the flow to prevent recursively inheriting `Inherited` when `PropagateStop` present, as opposed to not accessing `PropagateStop` entities entirely. Skips applying `C` when `Inherited<C>` and `PropagateOver<C>` are on the entity. ## Testing I updated the tests first to verify the test cases. They were incomplete and didn't pass once checking for the proper end state. After verifying those were not functioning, trimmed and adjusted code until it functioned. ## Fuller Explanation To my understanding of the goals (and my implementation) the idea is that the result of these components would be like ``` (Propagate<C>, Inherited<C>, C) (Inherited<C>, C) (PropagateOver<C>, Inherited<C>) (Inherited<C>, C) (PropagateStop<C>, Inherited<C>, C) ``` While the previous (incorrect by my measure) appeared to be ``` (Propagate<C>, Inherited<C>, C) (Inherited<C>, C) (PropagateOver<C>, Inherited<C>, C) (Inherited<C>, C) (PropagateStop<C>) ``` So of note: The `PropagateOver<C>` is still getting `Inherited<C>` in this current adjustment. I believe it was doing that before, but I didn't fully understand the reasoning for the `Inherited<C>` at all? I guess it's to properly mark that the `C` is an inherited one, so maybe I do need to remove it on `PropagateOver<C>` as well, to ensure cleanup doesn't also remove any separately applied `C`? I didn't look closely at what all the other tests are attempting to test (or if they are correctly testing those) to evaluate if other changes need to be made. if `PropagateOver<C>` doesn't have `Inherited<C>` and is reparented, would it be simple to ensure that its children have their `Inherited<C>` removed/updated properly? Looking at the code more, chances are this can be simplified a lot with the use of AncestorIter and DescendentIter, but I am not sure if jumping into a fuller refactor makes sense at this moment as opposed to attempting to fix this immediate issue and following up with a bigger refactor? thoughts? --------- Co-authored-by: robtfm <[email protected]>
1 parent d76c3aa commit 41bf46d

File tree

1 file changed

+383
-123
lines changed

1 file changed

+383
-123
lines changed

0 commit comments

Comments
 (0)