Controls: Fix Object contrast issue and tidy up code#33923
Conversation
|
View your CI Pipeline Execution ↗ for commit 967875e
☁️ Nx Cloud last updated this comment at |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR removes the getStyle-based dynamic styling system from JSON tree components, replacing it with CSS class–based styling and removing Theme/getStyle types and helpers. It also changes one ARIA role from "region" to "group" and includes minor manifest/package metadata edits. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/addons/docs/src/blocks/controls/Object.tsx`:
- Line 9: Remove the now-unused useTheme import and its call: delete "useTheme"
from the import statement (styled, useTheme) and remove the "const theme =
useTheme()" declaration inside the Object component (left over after removing
getCustomStyleFunction) so no dead reference remains.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/addons/docs/src/blocks/controls/Object.tsxcode/addons/docs/src/blocks/controls/react-editable-json-tree/JsonNodeAccordion.tsxcode/addons/docs/src/blocks/controls/react-editable-json-tree/JsonNodes.tsxcode/addons/docs/src/blocks/controls/react-editable-json-tree/index.tsx
💤 Files with no reviewable changes (1)
- code/addons/docs/src/blocks/controls/react-editable-json-tree/index.tsx
|
|
||
| import { cloneDeep } from 'es-toolkit/object'; | ||
| import { type Theme, styled, useTheme } from 'storybook/theming'; | ||
| import { styled, useTheme } from 'storybook/theming'; |
There was a problem hiding this comment.
useTheme import is now unused — dead code from removed getCustomStyleFunction.
const theme = useTheme() is declared on Line 166 but theme is never referenced anywhere in the component body after the getCustomStyleFunction removal. Both the import and the variable declaration should be removed.
🧹 Proposed fix
-import { styled, useTheme } from 'storybook/theming';
+import { styled } from 'storybook/theming'; export const ObjectControl: FC<ObjectProps> = ({ name, value, onChange, argType }) => {
- const theme = useTheme();
const data = useMemo(() => value && cloneDeep(value), [value]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/addons/docs/src/blocks/controls/Object.tsx` at line 9, Remove the
now-unused useTheme import and its call: delete "useTheme" from the import
statement (styled, useTheme) and remove the "const theme = useTheme()"
declaration inside the Object component (left over after removing
getCustomStyleFunction) so no dead reference remains.
yannbf
left a comment
There was a problem hiding this comment.
As long as Chromatic doesn't detect regressions (which I hope we have stories for) LGTM
17dd819 to
967875e
Compare
What I did
Caught an a11y issue in a contributor's UI Tests that had nothing to do with them, so opening a PR to fix it.
I've used the opportunity to remove a redundant styling API for Object control internals.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Documentation
ø
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Accessibility
Refactor