🐛 Token remap preserves child component sync after renaming a token group#9566
Open
RenzoMXD wants to merge 2 commits into
Open
🐛 Token remap preserves child component sync after renaming a token group#9566RenzoMXD wants to merge 2 commits into
RenzoMXD wants to merge 2 commits into
Conversation
Contributor
|
Thanks for your contribution! We've received your pull request and assigned it for review. We have a lot on our plate right now, so it may take us a little while to get to it but we appreciate your patience and will be in touch as soon as we can! |
…495-token-remap-touched-copy # Conflicts: # CHANGES.md
Contributor
Author
|
Hi, @niwinz |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #9495: after renaming a token group and clicking REMAP, applying a new token to the main component no longer propagates to its copies (child components).
Root cause
remap-tokens(frontend/src/app/main/data/workspace/tokens/remapping.cljs) updates applied-token references on shapes viapcb/update-shapes— but without:ignore-touched true. On a copy shape,set-shape-attrthen seesin-copy? = trueand a non-empty diff on:applied-tokens, so it flips the corresponding sync group (e.g.:fill-group) into the copy's:touchedset.Once a sync group is touched, the next main→copy sync skips that group in
generate-update-tokens(common/src/app/common/logic/libraries.cljc:1647), and the copy is silently left behind. This matches the bug report exactly: Step 5 works (initial apply was not touched), Step 7 fails (the copy is now touched on:fill-group).Fix
Pass
{:ignore-touched true}to thepcb/update-shapescall so REMAP — a refactor, not a user override — does not mark copies as touched.To keep the fix testable, the change-building portion of
remap-tokensis extracted into a purebuild-remap-changesfunction; the ptk event becomes a thin wrapper around it.Tests
Three new tests in
frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs:test-remap-tokens-finds-both-main-and-copy— scan finds the applied-token on both main and copy.test-remap-tokens-does-not-touch-copy— after REMAP, the copy's:touchedset does not contain:fill-group.test-remap-preserves-copy-sync-of-later-token-apply— end-to-end regression mirroring Steps 1–8 of the issue: build file → rename group → REMAP → applycolors.secondaryto main → run component sync → assert copy reflectscolors.secondary.All three tests were verified to fail without the fix and pass with it. The pre-fix E2E failure was:
— exactly the user-visible symptom in the issue.
Test plan
pnpm -C frontend run test— 3 new tests pass; no new failures (2 pre-existing unrelated typography-WASM failures and 1 pre-existingdisplay-guides-handlererror remain unchanged)