refactor(styles): theme-adaptive shared code theme#8023
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Storybook documentation page covering CodeHelp setup and embedded states, and fixes missing Button imports. Refactors Highlight to centralise raw HTML selection and escaping. Replaces hardcoded Highlight.js colours with design-system tokens and adds an embedded modifier. The onboarding code card now uses that modifier, while its local syntax-colour rules are removed. Estimated code review effort: 4 (Complex) | ~45 minutes 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 |
Docker builds report
|
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 541e0da9-4ac1-4a1d-bfe5-aae4eb6f2199
📒 Files selected for processing (6)
frontend/documentation/pages/CodeHelp.stories.tsxfrontend/web/components/CodeHelp.tsxfrontend/web/components/Highlight.jsfrontend/web/components/pages/onboarding/OnboardingConnectPanel/CodeCard.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/OnboardingConnectPanel.scssfrontend/web/styles/3rdParty/_hljs.scss
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
dba3f38 to
b1c981e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/web/components/Highlight.js (1)
3-3: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse the permitted component import path.
Relative import still present; this was already flagged in a prior review round.
Proposed fix
-import Button from './base/forms/Button' +import Button from 'components/base/forms/Button'As per coding guidelines, "Use only
common/,components/, andproject/import paths; do not use relative imports."Source: Coding guidelines
frontend/web/styles/3rdParty/_hljs.scss (1)
86-86: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore stylelint blank-line-before-comment compliance.
Still failing
scss/double-slash-comment-empty-line-beforeat all three sites from the previous round.Proposed fix
padding: 0.5em; + // Theme-adaptive surface: light in light mode, dark in dark, via tokens.padding: 18px; + // Unmapped tokens inherit the base colour instead of the highlight.js greys;border-radius: 0; + // Reserve the scrollbar gutter so hover doesn't shift layout; !importantAlso applies to: 183-183, 290-290
Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 80921548-b746-48c3-b290-c6e8aca8b379
📒 Files selected for processing (6)
frontend/documentation/pages/CodeHelp.stories.tsxfrontend/web/components/CodeHelp.tsxfrontend/web/components/Highlight.jsfrontend/web/components/pages/onboarding/OnboardingConnectPanel/CodeCard.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/OnboardingConnectPanel.scssfrontend/web/styles/3rdParty/_hljs.scss
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)
frontend/web/styles/3rdParty/_hljs.scss (1)
297-302: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid forcing the scrollbar on non-hover states
overflow-x: scrollplus the unqualified::-webkit-scrollbarrule keeps the bar visible even when the code fits. Useoverflow-x: auto, addscrollbar-gutter: stable, and limitdisplay: block !importantto the:hoverselector.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1505510c-624e-4fc5-8983-04c18188aa43
📒 Files selected for processing (1)
frontend/web/styles/3rdParty/_hljs.scss
ee08b16 to
abaf49d
Compare
abaf49d to
de8ab3a
Compare
de8ab3a to
b43e6bd
Compare
talissoncosta
left a comment
There was a problem hiding this comment.
Self-review notes to speed this up.
| </tr> | ||
| </tbody> | ||
| </table> | ||
| <h3>Colour: code</h3> |
There was a problem hiding this comment.
This is just documentation
talissoncosta
left a comment
There was a problem hiding this comment.
Self-review notes to speed this up.
| // Redefine Pylon's --hljs-* vars to our code tokens so its injected rules | ||
| // paint our theme (belt-and-braces with the palette specificity below). | ||
| --hljs-bg: var(--color-surface-subtle); | ||
| --hljs-text: var(--color-code-text); |
There was a problem hiding this comment.
Pylon (the support-chat widget) ships its own code-colour CSS that would override ours. These lines point its colour variables at our palette, so code stays on our theme. Only happens on deployed envs where Pylon loads.
| } | ||
| // Nested under .hljs (0,2,0) to out-specify Pylon's global token rules | ||
| // (0,1,0). The catch-all sends unmapped tokens to base; groups re-colour. | ||
| [class*='hljs-'] { |
There was a problem hiding this comment.
Pylon's rules and ours style the same elements; nesting ours here makes the browser prefer ours. The first rule resets any token we don't explicitly colour back to the base text colour.
|
|
||
| // The value to render before escaping: the live edit while focused, the | ||
| // current value when there's content, otherwise a disabled/empty placeholder. | ||
| getRawHtml = () => { |
There was a problem hiding this comment.
Pure refactor of an old nested ternary, no behaviour change.
New --color-code-* semantic family for highlight.js syntax, mapped to the nearest palette primitives (light in :root, lighter in .dark). Added to tokens.json (source of truth) and regenerated; registered in the Semantic Colour Tokens story so it shows the group. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Point the shared .hljs palette (and the Pylon --hljs-* var-feed) at the --color-code-* tokens so code blocks follow light/dark. Add .hljs--embedded for containers that own their surface (onboarding's CodeCard opts in via Highlight's embedded prop and drops its bespoke palette) and a theme-adaptive scrollbar. Nest the palette under .hljs to out-specify Pylon's injected rules. Drop the dead $code-color/$code-bg vars and the legacy .yaml override. Header chrome untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…awHtml Add an `embedded` boolean prop that applies the .hljs--embedded modifier, instead of callers concatenating the class into the language string. Import Button (via the components/ alias) instead of the global, since the Highlight story renders it and Storybook doesn't stub the global. Extract the nested raw-html ternary into a getRawHtml method. No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover the code wrapper across languages plus the embedded variant. Stub the isMobile global so components that read it render in Storybook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review (Kyle): red tag names read as errors. Tags now render like an IDE: grey angle brackets (base text via the catch-all), blue tag name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers markup tags inside JavaScript, mirroring the React code-help snippet, so the tag colour renders in Chromatic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shared React init snippet was two files mashed into one - two 'export default function' (App and HomePage, invalid JS), duplicate imports, an unused useFlagsmith import, and an App that never rendered HomePage. Make it one coherent example: a named HomePage using useFlags, rendered inside the default App wrapped in FlagsmithProvider. Shared via codeHelp.INIT, so this fixes the snippet app-wide, including the onboarding wire step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ports the onboarding code-card fix (was on #7989) to the shared embedded variant: overflow-x auto instead of a permanently reserved gutter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Snippet sources were pre-escaped (< >) because CodeHelp rendered them as raw HTML via preventEscape, so Copy Code shipped the entities to the clipboard. Store plain < > and let Highlight's default escaping handle rendering, like the onboarding code card already does. preventEscape now has no users, so the prop is gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewing this
Code snippets in the app (SDK examples, flag values) now switch with light/dark mode instead of always being light. Almost all of it is CSS in
_tokens.scss+_hljs.scss; the other files just use it. The only fiddly part: making our colours win over the support-chat widget (Pylon), which injects its own code CSS, and only on deployed envs. The accordion header above code blocks is left unchanged.Now also carries the rest of the code-snippet work (moved off #7989 plus Slack review feedback): markup tag colours, the corrected React snippet, and Copy Code no longer pasting
<entities.docs/if required so people know about the feature.Changes
Code snippets in the app (SDK examples, flag values, etc.) now follow light/dark mode instead of always being dark.
--color-code-*colour tokens for syntax, separate from the UI text colours< >brackets stay grey, like an IDEFlagsmithProvider+useFlags< >: snippet sources were pre-escaped HTML entities, so pasting into an editor produced<FlagsmithProviderHow did you test this code?
Toggle light/dark and check code snippets are readable in both (not always dark):
onboarding_quickstart_flow); scrollbar only when the snippet overflowsFlagsmithProvider+useFlags; tag names blue,< >grey< >, no<entitiesOptional (Storybook): from the
frontend/directory runnpm run storybook(serves on http://localhost:6006), then check theComponents/Highlightstories, both in light + dark.Plus CI visual-regression (light + dark).
lint+typecheckpass.Screenshots