Checklist: Fix contrast on dark mode and wait for checklist widget to load in stories #33121
Conversation
📝 WalkthroughWalkthroughAdded a stable DOM id to the Checklist widget, introduced a Storybook test helper that waits for that element, updated many sidebar stories to use the helper instead of fixed delays, and adjusted dark-mode theming for several Checklist UI elements. Changes
Sequence Diagram(s)sequenceDiagram
participant Story as Storybook Story (play)
participant Helper as waitForChecklistWidget
participant DOM as Document / ChecklistWidget
Story->>Helper: call waitForChecklistWidget()
Helper->>DOM: query `#storybook-checklist-widget`
alt element becomes visible within 5s
DOM-->>Helper: visible
Helper-->>Story: resolve (proceed)
else timeout (5s)
Helper-->>Story: reject (timeout)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
✨ Finishing touches
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (3)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/core/src/manager/components/sidebar/Sidebar.stories.tsx (1)
11-12: MakewaitForChecklistWidgetresilient to missingcheckVisibility()Centralizing the wait logic via
waitForChecklistWidgetis the right direction and aligns well with the newstorybook-checklist-widgetid. One concern:HTMLElement.prototype.checkVisibilityisn’t universally available (e.g., some jsdom-like environments), sodocument.getElementById(... )?.checkVisibility()may always yieldundefined, causingwaitForto time out even when the element is in the DOM.You can keep the same behavior but add a simple fallback when
checkVisibilityis not present:-const waitForChecklistWidget = () => - waitFor( - () => - expect(document.getElementById('storybook-checklist-widget')?.checkVisibility()).toBe(true), - { timeout: 5000 } - ); +const waitForChecklistWidget = () => + waitFor( + () => { + const el = document.getElementById('storybook-checklist-widget'); + expect(el).not.toBeNull(); + + const anyEl = el as any; + const isVisible = + typeof anyEl.checkVisibility === 'function' + ? anyEl.checkVisibility() + : !!el!.offsetParent; + + expect(isVisible).toBe(true); + }, + { timeout: 5000 } + );This keeps the intent (wait until the widget is actually visible) while avoiding hard failures in environments that don’t implement
checkVisibility.Also applies to: 185-191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/core/src/manager/components/sidebar/ChecklistWidget.tsx(1 hunks)code/core/src/manager/components/sidebar/Sidebar.stories.tsx(11 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager/components/sidebar/ChecklistWidget.tsxcode/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/components/sidebar/ChecklistWidget.tsxcode/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/core/src/manager/components/sidebar/ChecklistWidget.tsx (1)
82-87: IDs for widget and collapse toggle are appropriate for test hooksAdding
id="storybook-checklist-widget"onHoverCardand wiring#checklist-module-collapse-toggleinto the hover CSS cleanly exposes the widget and toggle for tests without changing runtime behavior. Assuming there is only one checklist widget per page (which matches Sidebar usage), this looks good.Also applies to: 194-217
code/core/src/manager/components/sidebar/Sidebar.stories.tsx (1)
192-194: Consistently waiting for the checklist widget across stories improves VRT stabilityWiring
play: waitForChecklistWidgetinto the various Sidebar stories (Simple/Mobile/Empty/WithRefs/Statuses/Searching/Scrolled, etc.) is a good way to eliminate the previous flakiness from the checklist’s delayed rendering, and replacing the hard-coded wait inSearching/Scrolledwithawait waitForChecklistWidget()makes the plays more robust and intention-revealing. No issues spotted with the wayplayis assigned or awaited.Also applies to: 209-213, 228-233, 235-240, 242-251, 253-258, 260-265, 267-302, 304-309, 331-337, 339-371, 373-403, 405-423, 495-497
|
View your CI Pipeline Execution ↗ for commit e07f0bc
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/core/src/manager/components/sidebar/Sidebar.stories.tsx (1)
185-190: Improve error messaging and consider browser compatibility.The optional chaining combined with
expect(...).toBe(true)will produce unclear error messages when the element doesn't exist. IfgetElementByIdreturnsnull, the expression evaluates toexpect(undefined).toBe(true), which doesn't clearly indicate whether the element is missing or not visible. Additionally,checkVisibility()is a relatively new DOM API that may not be supported in all test environments.Consider this refactor for clearer error messages:
-const waitForChecklistWidget = () => - waitFor( - () => - expect(document.getElementById('storybook-checklist-widget')?.checkVisibility()).toBe(true), - { timeout: 5000 } - ); +const waitForChecklistWidget = () => + waitFor( + () => { + const element = document.getElementById('storybook-checklist-widget'); + expect(element).not.toBeNull(); + expect(element!.checkVisibility()).toBe(true); + }, + { timeout: 5000 } + );Alternatively, for better browser compatibility, use a more widely supported visibility check:
-const waitForChecklistWidget = () => - waitFor( - () => - expect(document.getElementById('storybook-checklist-widget')?.checkVisibility()).toBe(true), - { timeout: 5000 } - ); +const waitForChecklistWidget = () => + waitFor( + () => { + const element = document.getElementById('storybook-checklist-widget'); + expect(element).not.toBeNull(); + expect(element!.offsetParent).not.toBeNull(); + }, + { timeout: 5000 } + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/manager/components/sidebar/Sidebar.stories.tsx(13 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/core/src/manager/components/sidebar/Sidebar.stories.tsx (3)
11-11: LGTM!The
waitForimport is correctly added and used by the new helper function.
193-193: LGTM! Consistent application of the checklist widget wait.The
waitForChecklistWidgethelper is consistently applied across all relevant stories. Stories that shouldn't wait for the widget (Loading variants, SimpleInProduction, Bottom) correctly omit the play hook. The inline implementations in Searching and Scrolled properly await the helper before proceeding with their test steps.Also applies to: 212-212, 232-232, 239-239, 250-250, 257-257, 264-264, 301-301, 308-308, 336-336, 370-370, 402-402, 417-417, 496-496
282-282: Verify the viewport dimension changes.The width and viewport values for the
narrowconfiguration have been modified. Are these changes intentional, or were they incidental edits? The PR objective focuses on waiting for the checklist widget, so these dimension changes seem unrelated.Also applies to: 291-291
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/core/src/manager/components/sidebar/Sidebar.stories.tsx (2)
11-12: GuardcheckVisibilitycall for non-supporting environments
waitForChecklistWidgetonly optional-chains the element lookup; once the element exists,el.checkVisibility()is called unconditionally. In environments that don’t yet implementElement.prototype.checkVisibility(e.g., some non-browser/DOM-like test environments), this will throw aTypeErrorrather than just causing the assertion to fail, andwaitForwill keep retrying with the same error until timeout.To make this helper more robust without changing its intent, consider guarding the method call and falling back to a simpler presence check:
const waitForChecklistWidget = async () => { await waitFor(() => { const el = document.getElementById('storybook-checklist-widget'); expect(el).not.toBeNull(); if (el && 'checkVisibility' in el) { // Cast if needed depending on your DOM lib version. expect((el as any).checkVisibility()).toBe(true); } }, { timeout: 5000 }); await wait(300); // wait for expand animation };This still waits for the widget to be rendered/visible where supported, but won’t hard-crash stories/tests in environments that lack
checkVisibility.Also applies to: 185-192
279-295: KeepWithRefsNarrowwidth and Chromatic viewport values coupledAligning the CSS width (
230px) with the Chromatic mode viewport (viewport: 230) is sensible and should give more predictable screenshots for this narrow layout.If this value ever needs to change again, consider extracting it into a small shared constant (even just within this story object) to avoid the two numbers drifting out of sync.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/manager/components/sidebar/Sidebar.stories.tsx(13 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/core/src/manager/components/sidebar/Sidebar.stories.tsx (2)
194-196: Good consolidation of checklist-wait logic across relevant storiesWiring
play: waitForChecklistWidgetinto the stories that actually render the checklist widget (Simple, Mobile, Empty*, WithRefs*, Statuses*, etc.) is a nice improvement over ad-hoc fixed delays. It centralizes the timing behavior, makes the stories’ intent clearer, and should materially reduce the VRT flakiness you’re targeting, while keeping non-checklist stories (Loading*, ref error variants, Bottom) untouched.No issues from my side on these play additions.
Also applies to: 211-215, 230-235, 237-242, 244-253, 255-260, 262-267, 269-304, 306-311, 333-339, 341-373, 375-405
407-425: Sequencing interactions after checklist readiness looks solidIn both
SearchingandScrolled, insertingawait waitForChecklistWidget()at the start of theplayfunction ensures all subsequent interactions (typing into search, expanding items, scrolling, toggling state) happen only after the checklist widget has mounted and finished its expand animation. That matches the PR goal of de-flaking sidebar visuals tied to the widget’s layout.The rest of the play logic remains unchanged in behavior, so this should be a safe stabilization.
Also applies to: 463-520
ghengeveld
changed the title
| return ( | ||
| <CollapsibleWithMargin collapsed={!hasItems || !loaded}> | ||
| <HoverCard outlineAnimation="rainbow"> | ||
| <HoverCard id="storybook-checklist-widget" outlineAnimation="rainbow"> |
There was a problem hiding this comment.
If the ID is purely for testing purposes, we should prefer to use something like data-test-id instead. For the future
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
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
Tests
Chores
Style
✏️ Tip: You can customize this high-level summary in your review settings.