fix: key toolbar marker and length units on config, not label text - #89
Open
FutuRiata wants to merge 1 commit into
Open
fix: key toolbar marker and length units on config, not label text#89FutuRiata wants to merge 1 commit into
FutuRiata wants to merge 1 commit into
Conversation
Two places decided behaviour by comparing the visible label text. Both break silently as soon as the UI is translated: no error, no console output. The Fillet toolbar entry now carries an optional `tool` id and the button reads `data-sketchforge-tool` from it instead of matching `label === "Fillet"`, so the nameplate tutorial coachmark keeps finding the button whatever the label says. `RangePropertyConfig` gains optional `unit` and `allowsAboveSliderMax` fields. `RangeProperty` prefers them and falls back to the existing label lists when they are absent, so property declarations that do not set them behave exactly as before. The 45 length properties are annotated to resolve to the same values the label rules produce today, verified declaration by declaration. No user-visible change: the rendered attribute, the displayed units and the slider limits are identical.
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
1 participant
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.
Two places decide behaviour by comparing the visible label text. Both break silently as soon as the UI is translated — no error, no console output, so they are easy to miss.
1. Toolbar button marker
SketchForgeEditor.tsxsetsdata-sketchforge-toolwhenlabel === "Fillet", andNameplateTutorialPanel.tsxfinds that button through the attribute, bailing out withif (!target) return. Translate the label and the tutorial coachmark silently stops appearing — the step still works, but the pointer never shows up.The Fillet toolbar entry now carries an optional
toolid and the button reads the attribute from it. Two lines, no behaviour change.2. Length units in the shape inspector
propertyUsesLengthUnit(label)matches the label against a list of fifteen English words to decide whether to show measurement units, and a second comparison decides whether a value may exceed the slider maximum. Together these make the properties panel untranslatable: translated labels silently lose their units.RangePropertyConfiggains optionalunitandallowsAboveSliderMaxfields.RangePropertyprefers them and falls back to the existing label lists when they are absent, so declarations that do not set them behave exactly as before and the change can land incrementally. The 45 length properties are annotated to resolve to the same values the label rules produce today, verified declaration by declaration.Verification
npm test— 264 tests pass.Context
I maintain a Russian fork and hit both of these. The fork itself keeps all of its changes outside your files — the translation is applied at build time — but these two spots cannot be worked around that way, because the behaviour genuinely depends on the English text. They affect anyone translating the editor into any language.
Happy to adjust the approach or split this into two PRs if you prefer.