feat: format action for JSON feature values - #8389
Conversation
The .json toggle only validates and highlights; the stored value is rendered verbatim, so pasted minified JSON stays unreadable. Add a Format action next to copy, shown only when the language is JSON and there is a value, that pretty-prints via JSON.stringify(parse, 2). It calls the same onChange the editor already uses for typing, so every caller (feature value, variations, segment overrides) gets it for free. Invalid JSON is left untouched with a toast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthrough
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR adds a JSON-only formatting action that preserves invalid values and uses the existing value-update path. Merge readiness is otherwise unaffected; only a minor stylesheet lint fix remains before normal checks can pass. 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 |
github-actions
Bot
added
the
front-end
The .primary span already sets color: var(--color-text-action); Icon falls back to currentColor, so the hardcoded #6837fc was redundant and not theme-aware. Dropping it keeps the same colour and adapts in dark mode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
feature
Replace the clickable span with BareButton so the action is a real button: keyboard focusable, focus-visible outline, disabled semantics. Extend the .select-language selector to style .bare-btn alongside span. Animate the show/hide with a max-width + opacity transition (collapsing out of the row when not JSON) instead of popping in, and honour prefers-reduced-motion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
feature
Only preventDefault is needed, to keep the editor from blurring (which would fire its onBlur commit). stopPropagation was copied from the tab spans with no handler here to stop, and can interfere with click-away. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
feature
formatJson emitted onChange unconditionally, and the feature modal flags the value dirty on any onChange without comparing. So formatting an already-formatted value showed a spurious pending change. Skip onChange when the reformatted string is identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7998c93b-b070-496b-9dc5-26923834aba7
📒 Files selected for processing (2)
frontend/web/components/ValueEditor.jsfrontend/web/styles/3rdParty/_hljs.scss
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| color: var(--color-text-action); | ||
| } | ||
| } | ||
| // Slides and fades in when JSON is selected, collapses out otherwise. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required blank line before the new comment.
Stylelint reports scss/double-slash-comment-empty-line-before because the comment on Line 85 follows the closing brace on Line 84 without an empty line. Insert one blank line so the changed stylesheet passes the configured lint rule.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 85-85: Expected empty line before comment (scss/double-slash-comment-empty-line-before)
(scss/double-slash-comment-empty-line-before)
Source: Linters/SAST tools
1 participant
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
The
.jsontoggle on the value editor only validates and syntax-highlights; the stored value is rendered verbatim. So a pasted, minified JSON blob stays on one unreadable line with no way to indent it.Adds a Format action next to
copy, shown only when the language is JSON and there is a value. It pretty-prints viaJSON.stringify(JSON.parse(value), null, 2)and calls the sameonChangethe editor already uses for typing, so every consumer ofValueEditor(feature value, multivariate variations, segment overrides) gets it for free. Invalid JSON is left untouched with a toast.Draft: raising for a quick design/product check before finalising.
Open questions for the team:
formattext +codeicon, or an icon-only button?JSON.parseconsumers.)How did you test this code?
Not yet run in a live app — this is a draft for design/product validation. What I have done:
eslint --fixpasses (the one remaining!=warning is pre-existing, on an untouched line).onChangecontract by tracing callers:Highlightalready emits a plain string toonChange, and callers run it throughUtils.safeParseEventValue, so a formatted string follows the exact same path as typed input.Suggested manual test before merge:
.jsontab.