feat: allow value feedbacks as layered-button style overrides - #4379
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds typed feedback style overrides with value-feedback context. It updates validation, entity pools, graphics evaluation, and Web UI editing for ChangesFeedback style override flow
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
77acc09 to
df91a66
Compare
Value feedbacks were blocked from the layered-button style override list; only boolean and advanced feedbacks were permitted. This exposes a value feedback's value to its style overrides as an expression, so a user can override an element property with a transform of the feedback value. - Allow Value feedbacks in the StyleOverride list (canAddEntityToFeedbackList) - Carry the feedback value through the override map via a new ResolvedFeedbackStyleOverride shape (thisContext), threaded through the entity pools, drawer and graphics element conversion - Evaluate value-feedback override expressions in ElementExpressionHelper with $(this:value) bound to the feedback value, tracking referenced variables; an undefined result means "no override" (base value is used) - UI: value-feedback overrides are always edited as an expression, seeded with $(this:value), with that value offered in the picker - Add a feedbackStyleOverridesUnsupported definition flag so a feedback can opt out of the style-override list entirely (menu + validation); apply it to the internal user_value feedback, which is confusing as a style override - Show an icon with hover text in the style-override editor indicating how the placed feedback type (value/boolean/advanced) interacts with overrides - Add unit tests across shared-lib, companion and webui Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011dJegJRRd8hCL62tzmy1ft
df91a66 to
1d2c488
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webui/src/Controls/Components/LayeredStylesOverrides.tsx (1)
388-437: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a focused test for the value-feedback editor path.
The current tests cover helper output only. Add a unit test that verifies a value-feedback edit saves
{ isExpression: true }and exposesthis:valuein the expression variables. This protects the new resolver contract from a literal-value regression.As per coding guidelines, “When changing behavior, improve unit-test coverage rather than relying on manual testing alone.”
Source: Coding guidelines
🧹 Nitpick comments (1)
webui/src/Controls/Components/LayeredStylesOverrides.tsx (1)
47-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the exported helpers to a non-component module.
react-refresh/only-export-componentsreports warnings fordefaultStyleOverrideValueandfeedbackTypeInteractionHelp. KeepLayeredStylesOverrides.tsxlimited to component exports. Update the test import to use the new helper module.As per coding guidelines, “Before finishing a change, ensure
yarn check-types,yarn lint, andyarn vitest runpass.”Sources: Coding guidelines, Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: af37d5ca-cf4f-41dc-a86e-88b2125a4c48
📒 Files selected for processing (24)
companion/lib/Controls/ControlTypes/Button/LayeredButtonDrawer.tscompanion/lib/Controls/Entities/EntityListPoolBase.tscompanion/lib/Controls/Entities/EntityListPoolButton.tscompanion/lib/Controls/Entities/EntityListPoolExpressionVariable.tscompanion/lib/Controls/Entities/EntityListPoolPage.tscompanion/lib/Controls/Entities/EntityListPoolTrigger.tscompanion/lib/Graphics/ConvertGraphicsElements.tscompanion/lib/Graphics/ConvertGraphicsElements/Helper.tscompanion/lib/Instance/Connection/ChildHandlerLegacy.tscompanion/lib/Instance/Connection/Thread/HostContext.tscompanion/lib/Internal/Controller.tscompanion/lib/Internal/Variables.tscompanion/test/Controls/Entities/EntityListPool.test.tscompanion/test/Graphics/ConvertGraphicsElements.test.tscompanion/test/Graphics/ConvertGraphicsElements/Helper.test.tsshared-lib/lib/Entity.tsshared-lib/lib/Model/EntityDefinitionModel.tsshared-lib/lib/Model/EntityModel.tsshared-lib/lib/__tests__/Entity.test.tswebui/src/Controls/Components/EntityCommonCells.tsxwebui/src/Controls/Components/LayeredStylesOverrides.tsxwebui/src/Controls/Components/__tests__/LayeredStylesOverrides.test.tswebui/src/Controls/LocalVariablesStore.tsxwebui/src/scss/_button-edit.scss
- getParsedString: a value-feedback override that resolves to undefined now falls back to the element's own value AND variable-interpolates it (a string property is a template), instead of rendering the raw $(...) text - Update the style-override variable picker to the new getOptions signature (actionContext: NotActions), fixing the check-types CI failure Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011dJegJRRd8hCL62tzmy1ft
The rebase dropped main's additions to _button-edit.scss (the wysiwyg toolbar from #4341 and the value-feedback overrides from #4379) as part of resolving the convert-to-CSS modify/delete. Re-apply them in button-edit.css: the canvas workspace + quick-actions/aspect toolbars, the panel resize bar with its mode-toggle, the elementlist heading/body split, the fixed layered- overrides table layout and the portaled aspect-custom popover. New colours go through tokens (a button-editor toolbar palette + shadow-4/-6 for the subtle lifts); the rest reuse existing surface/border/primary tokens.
The rebase dropped main's additions to _button-edit.scss (the wysiwyg toolbar from #4341 and the value-feedback overrides from #4379) as part of resolving the convert-to-CSS modify/delete. Re-apply them in button-edit.css: the canvas workspace + quick-actions/aspect toolbars, the panel resize bar with its mode-toggle, the elementlist heading/body split, the fixed layered- overrides table layout and the portaled aspect-custom popover. New colours go through tokens (a button-editor toolbar palette + shadow-4/-6 for the subtle lifts); the rest reuse existing surface/border/primary tokens.
The rebase dropped main's additions to _button-edit.scss (the wysiwyg toolbar from #4341 and the value-feedback overrides from #4379) as part of resolving the convert-to-CSS modify/delete. Re-apply them in button-edit.css: the canvas workspace + quick-actions/aspect toolbars, the panel resize bar with its mode-toggle, the elementlist heading/body split, the fixed layered- overrides table layout and the portaled aspect-custom popover. New colours go through tokens (a button-editor toolbar palette + shadow-4/-6 for the subtle lifts); the rest reuse existing surface/border/primary tokens.
An absurd example:
Which results in the feedback applying every other second, setting the button text to the
abc 05:22:20when it is activeAlso adds some tooltips, which I am not 100% happy with placement/style of, but I think necessary to have somewhere:
Summary by CodeRabbit
New Features
Style