Core: Track vision simulator state through globals and apply styles in preview#33418
Conversation
…ers in the preview rather than on the manager's preview iframe
|
View your CI Pipeline Execution ↗ for commit e3601ea
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughRemoves the inline SVG filter component, centralizes vision filter metadata and SVG defs, migrates VisionSimulator state to Storybook globals, adds a decorator that injects/applies SVG/CSS filters to the DOM, and wires an initial global key and preview decorator. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as VisionSimulator UI
participant Globals as Storybook Globals
participant Decorator as withVisionSimulator
participant DOM as Document/Body
User->>UI: open selector / choose filter
UI->>Globals: set global `vision` value
Globals->>Decorator: globals change (vision)
rect rgb(240,248,255)
Note over Decorator,DOM: On mount
Decorator->>DOM: inject hidden element with `filterDefs` (SVG filters)
end
rect rgb(240,255,240)
Note over Decorator: On vision change
Decorator->>Decorator: compute merged body filter string (preserve other filters)
Decorator->>DOM: update `document.body.style.filter`
end
rect rgb(255,240,245)
Note over Decorator,DOM: On unmount / cleanup
Decorator->>DOM: remove injected filter element
Decorator->>DOM: restore previous `body.style.filter`
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (8)**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,jsx,json,html,ts,tsx,mjs}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{test,spec}.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (19)📓 Common learnings📚 Learning: 2025-11-05T09:36:55.944ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-12-22T22:03:40.123ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:31.838ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:31.838ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-12-22T22:03:40.123ZApplied to files:
📚 Learning: 2025-11-24T17:49:59.279ZApplied to files:
📚 Learning: 2025-12-22T22:03:40.123ZApplied to files:
⏰ 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 (4)
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: 2
🧹 Nitpick comments (1)
code/addons/a11y/src/withVisionSimulator.ts (1)
20-33: Handle empty filter strings correctly.When
document.body.style.filteris an empty string,split(' ')returns[''], which passes through the filter check on line 22. While this won't break functionality (empty strings are filtered out by the truthiness check), it's worth being explicit.🔎 Optional clarification
useEffect(() => { - const existingFilters = document.body.style.filter.split(' ').filter((filter) => { + const existingFilters = (document.body.style.filter || '').split(' ').filter((filter) => { return filter && filter !== 'none' && !knownFilters.includes(filter); });This makes the intent clearer and handles the empty string case explicitly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
code/addons/a11y/src/components/ColorFilters.tsxcode/addons/a11y/src/components/VisionSimulator.tsxcode/addons/a11y/src/constants.tscode/addons/a11y/src/preview.tsxcode/addons/a11y/src/visionSimulatorFilters.tscode/addons/a11y/src/withVisionSimulator.ts
💤 Files with no reviewable changes (1)
- code/addons/a11y/src/components/ColorFilters.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/a11y/src/preview.tsxcode/addons/a11y/src/constants.tscode/addons/a11y/src/withVisionSimulator.tscode/addons/a11y/src/visionSimulatorFilters.tscode/addons/a11y/src/components/VisionSimulator.tsx
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/a11y/src/preview.tsxcode/addons/a11y/src/constants.tscode/addons/a11y/src/withVisionSimulator.tscode/addons/a11y/src/visionSimulatorFilters.tscode/addons/a11y/src/components/VisionSimulator.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/a11y/src/preview.tsxcode/addons/a11y/src/constants.tscode/addons/a11y/src/withVisionSimulator.tscode/addons/a11y/src/visionSimulatorFilters.tscode/addons/a11y/src/components/VisionSimulator.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/a11y/src/preview.tsxcode/addons/a11y/src/constants.tscode/addons/a11y/src/withVisionSimulator.tscode/addons/a11y/src/visionSimulatorFilters.tscode/addons/a11y/src/components/VisionSimulator.tsx
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/a11y/src/preview.tsxcode/addons/a11y/src/constants.tscode/addons/a11y/src/withVisionSimulator.tscode/addons/a11y/src/visionSimulatorFilters.tscode/addons/a11y/src/components/VisionSimulator.tsx
🧠 Learnings (13)
📚 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/addons/a11y/src/preview.tsxcode/addons/a11y/src/withVisionSimulator.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/addons/a11y/src/preview.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/addons/a11y/src/preview.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/addons/a11y/src/withVisionSimulator.tscode/addons/a11y/src/components/VisionSimulator.tsx
📚 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/addons/a11y/src/components/VisionSimulator.tsx
🧬 Code graph analysis (3)
code/addons/a11y/src/preview.tsx (1)
code/addons/a11y/src/withVisionSimulator.ts (1)
withVisionSimulator(9-36)
code/addons/a11y/src/withVisionSimulator.ts (2)
code/addons/a11y/src/visionSimulatorFilters.ts (2)
filters(1-46)filterDefs(48-100)code/frameworks/angular/src/client/public-types.ts (1)
StoryFn(35-38)
code/addons/a11y/src/components/VisionSimulator.tsx (2)
code/addons/a11y/src/visionSimulatorFilters.ts (2)
filters(1-46)filterDefs(48-100)code/addons/a11y/src/constants.ts (1)
VISION_GLOBAL_KEY(4-4)
🪛 ast-grep (0.40.3)
code/addons/a11y/src/components/VisionSimulator.tsx
[warning] 63-63: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
code/addons/a11y/src/components/VisionSimulator.tsx
[error] 64-64: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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). (4)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (9)
code/addons/a11y/src/components/VisionSimulator.tsx (3)
1-11: LGTM! Clean migration to global state.The imports have been properly updated to support the global state architecture. The transition from local state management to
useGlobalsis a solid architectural improvement that enables cross-component state synchronization.
46-51: LGTM! Options generation is clean and type-safe.The dynamic generation of options from the
filtersmap is well-structured and leverages TypeScript's type inference effectively.
64-64: The use ofdangerouslySetInnerHTMLis safe here.Static analysis tools flag this line, but the concern is a false positive. The
filterDefsconstant is a hardcoded SVG string fromvisionSimulatorFilters.ts, not user-supplied input, so there's no XSS risk. This is an appropriate use case for injecting static SVG filter definitions into the DOM.code/addons/a11y/src/constants.ts (1)
4-4: LGTM! Clean constant addition.The new
VISION_GLOBAL_KEYconstant properly centralizes the global state key for the vision simulator feature.code/addons/a11y/src/preview.tsx (2)
8-12: LGTM! Decorator integration is clean.The
withVisionSimulatordecorator is properly imported and exported in thedecoratorsarray, enabling vision simulation across all stories.
100-100: Thevisionglobal is correctly placed at the top level ofinitialGlobalsand is consistently accessed throughout the codebase via destructuring (const { vision } = globals) and bracket notation (globals[VISION_GLOBAL_KEY]where the key resolves to"vision"). The structure is intentional:a11y-related settings likemanualare nested, whilevisionis a separate top-level global.code/addons/a11y/src/withVisionSimulator.ts (1)
7-7: LGTM! Clean computation of known filters.The
knownFiltersarray is correctly derived from thefiltersmap, ensuring consistent filter detection throughout the decorator.code/addons/a11y/src/visionSimulatorFilters.ts (2)
1-46: LGTM! Well-structured filter definitions.The
filtersobject provides a clean, centralized mapping of vision deficiencies with their labels, CSS filter values, and prevalence percentages. The use ofas constensures type safety for the filter keys.
48-100: LGTM! SVG filter definitions are properly structured.The
filterDefsconstant contains well-formed SVG markup with color matrix filters for various vision deficiencies. The filter IDs correctly match the references in thefiltersobject, and thedisplay: none !importantstyle ensures the SVG doesn't affect layout.
| }, | ||
| ({ $filter }) => ({ | ||
| filter: getFilter($filter), | ||
| filter: filters[$filter as keyof typeof filters].filter || 'none', |
There was a problem hiding this comment.
Verify the filter key exists before accessing.
The code assumes the filter key exists in the filters object, but if $filter is invalid, this could throw at runtime.
🔎 Proposed defensive fix
({ $filter }) => ({
- filter: filters[$filter as keyof typeof filters].filter || 'none',
+ filter: filters[$filter as keyof typeof filters]?.filter || 'none',
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| filter: filters[$filter as keyof typeof filters].filter || 'none', | |
| ({ $filter }) => ({ | |
| filter: filters[$filter as keyof typeof filters]?.filter || 'none', | |
| }), |
🤖 Prompt for AI Agents
In code/addons/a11y/src/components/VisionSimulator.tsx around line 35, the
lookup filters[$filter as keyof typeof filters].filter assumes $filter is a
valid key and may throw if it's invalid; first check that $filter exists in the
filters object (e.g. use "if ($filter in filters)" or
Object.prototype.hasOwnProperty.call(filters, $filter)) and only access .filter
when present, otherwise fall back to 'none' (or use optional chaining like
filters[$filter as keyof typeof filters]?.filter ?? 'none'); ensure the check is
typed so TypeScript accepts the access and keep the 'none' fallback.
| useEffect(() => { | ||
| document.body.insertAdjacentHTML('beforeend', filterDefs); | ||
|
|
||
| return () => { | ||
| document.body.removeChild(document.getElementById('storybook-a11y-vision-filters')!); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Add defensive checks for DOM manipulation.
The cleanup function uses a non-null assertion when removing the filter element. If the element was already removed (e.g., by another story or external code), this will throw a runtime error. Additionally, if multiple stories mount simultaneously, insertAdjacentHTML could insert duplicate elements with the same ID.
🔎 Proposed fix with existence checks
useEffect(() => {
- document.body.insertAdjacentHTML('beforeend', filterDefs);
+ const existingFilters = document.getElementById('storybook-a11y-vision-filters');
+ if (!existingFilters) {
+ document.body.insertAdjacentHTML('beforeend', filterDefs);
+ }
return () => {
- document.body.removeChild(document.getElementById('storybook-a11y-vision-filters')!);
+ const element = document.getElementById('storybook-a11y-vision-filters');
+ if (element && element.parentNode) {
+ element.parentNode.removeChild(element);
+ }
};
}, []);🤖 Prompt for AI Agents
In code/addons/a11y/src/withVisionSimulator.ts around lines 12–18, the effect
unconditionally inserts filterDefs and uses a non-null assertion when removing
the element; change it to first check if an element with id
'storybook-a11y-vision-filters' already exists before inserting (skip insertion
if present) and in the cleanup use getElementById/querySelector to find the
element and only call removeChild if the element is found (no non-null
assertion). This prevents duplicate inserts when multiple stories mount and
avoids runtime errors if the element was removed elsewhere.
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
ghengeveld
force-pushed
the
vision-simulator-global
branch
from
9af6551 to
8bf98c1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
code/addons/a11y/src/components/VisionSimulator.tsx (1)
29-31: Add optional chaining for defensive access.This was flagged in a previous review but hasn't been addressed. If
$filteris not a valid key infilters, this will throw at runtime.🔎 Proposed fix
({ $filter }) => ({ - filter: filters[$filter as keyof typeof filters].filter || 'none', + filter: filters[$filter as keyof typeof filters]?.filter || 'none', }),
🧹 Nitpick comments (2)
code/addons/a11y/src/components/VisionSimulator.stories.tsx (1)
9-17: Consider adding a proper type for the mock context.Using
anybypasses TypeScript strict mode. While acceptable for test files, a partial type would improve safety.🔎 Proposed type-safe mock
-const managerContext: any = { +const managerContext: Partial<{ + state: Record<string, unknown>; + api: Record<string, ReturnType<typeof fn>>; +}> = { state: {}, api: { getGlobals: fn(() => ({ vision: undefined })), updateGlobals: fn(), getStoryGlobals: fn(() => ({ vision: undefined })), getUserGlobals: fn(() => ({ vision: undefined })), }, };code/addons/a11y/src/components/VisionSimulator.tsx (1)
59-59: Acknowledged:dangerouslySetInnerHTMLfor SVG filter definitions.Static analysis flagged this for XSS risk. However,
filterDefsis imported from a local module (../visionSimulatorFilters) containing hardcoded SVG filter definitions—not user input. The XSS risk is minimal since the content is developer-controlled source code.Consider adding a brief comment explaining why this is safe to suppress future linter warnings:
🔎 Proposed clarifying comment
+ {/* filterDefs contains static SVG filter definitions from visionSimulatorFilters.ts */} <Hidden dangerouslySetInnerHTML={{ __html: filterDefs }} />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.tsxcode/addons/a11y/src/withVisionSimulator.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/addons/a11y/src/withVisionSimulator.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.tsx
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.tsx
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.tsx
🧠 Learnings (6)
📓 Common learnings
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.
📚 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/addons/a11y/src/components/VisionSimulator.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/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.tsx
📚 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/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.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/addons/a11y/src/components/VisionSimulator.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/addons/a11y/src/components/VisionSimulator.stories.tsx
🪛 ast-grep (0.40.3)
code/addons/a11y/src/components/VisionSimulator.tsx
[warning] 58-58: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
code/addons/a11y/src/components/VisionSimulator.tsx
[error] 59-59: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (6)
code/addons/a11y/src/components/VisionSimulator.stories.tsx (3)
19-29: LGTM!The decorator pattern correctly provides the ManagerContext needed for the VisionSimulator component to function in isolation.
33-35: LGTM!Good use of role-based querying with
getByRolefor accessible and resilient interaction testing.
37-46: LGTM!The
WithFilterstory effectively tests the new globals-based vision filter state, which aligns with the PR objective of synchronizing vision simulator state with Storybook globals for Chromatic snapshot testing.code/addons/a11y/src/components/VisionSimulator.tsx (3)
1-11: LGTM!Imports are well-organized. Using
useGlobalsfromstorybook/manager-apicorrectly implements the global state migration.
37-46: LGTM!The refactor from local state to global state via
useGlobalsis clean. Deriving options from the centralizedfiltersmap improves maintainability.
55-55: ThedefaultOptionsprop usage is correct. The Select component's API explicitly supports passing either a single value or an array (defaultOptions?: Value | Value[]), so receiving a string key is valid and no change is needed.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
code/addons/a11y/src/withVisionSimulator.ts (1)
40-47: Prevent duplicate filter definition insertions.Line 41 unconditionally inserts
filterDefsinto the DOM, which can create duplicate elements with the same ID (storybook-a11y-vision-filters) if multiple stories mount simultaneously or if the decorator is applied multiple times.🔎 Proposed fix
useEffect(() => { - document.body.insertAdjacentHTML('beforeend', filterDefs); + const existingFilters = document.getElementById('storybook-a11y-vision-filters'); + if (!existingFilters) { + document.body.insertAdjacentHTML('beforeend', filterDefs); + } return () => { const filterDefsElement = document.getElementById('storybook-a11y-vision-filters'); filterDefsElement?.parentElement?.removeChild(filterDefsElement); }; }, []);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/a11y/src/withVisionSimulator.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/a11y/src/withVisionSimulator.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/a11y/src/withVisionSimulator.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/a11y/src/withVisionSimulator.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/a11y/src/withVisionSimulator.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/a11y/src/withVisionSimulator.ts
🧠 Learnings (6)
📓 Common learnings
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.
📚 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/addons/a11y/src/withVisionSimulator.ts
📚 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/addons/a11y/src/withVisionSimulator.ts
📚 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/addons/a11y/src/withVisionSimulator.ts
📚 Learning: 2025-11-05T09:36:55.944Z
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.
Applied to files:
code/addons/a11y/src/withVisionSimulator.ts
📚 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/addons/a11y/src/withVisionSimulator.ts
🧬 Code graph analysis (1)
code/addons/a11y/src/withVisionSimulator.ts (1)
code/addons/a11y/src/visionSimulatorFilters.ts (2)
filters(1-46)filterDefs(48-100)
⏰ 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)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
code/addons/a11y/src/components/VisionSimulator.stories.tsx (4)
1-1: Remove unused import.
PlayFunctionContextis imported but never used in this file.🔎 Proposed fix
-import type { PlayFunction, PlayFunctionContext } from 'storybook/internal/types'; +import type { PlayFunction } from 'storybook/internal/types';
9-17: Improve type safety and consider test isolation concerns.The
managerContextis typed asanyand defined at module scope, which could lead to:
- Reduced type safety.
- Test interference — the mock functions (
fn()) accumulate call history across all stories. If stories run sequentially or in parallel, assertions like line 52 (expect(managerContext.api.updateGlobals).toHaveBeenCalledWith(...)) could produce false positives or negatives.Consider typing the mock more strictly (e.g.,
Partial<ManagerContextProps>) and resetting or isolating mocks per story if call-count assertions are critical.🔎 Proposed improvements
Option 1: Add explicit type annotation
import type { ManagerContextProps } from 'storybook/manager-api'; const managerContext: Partial<ManagerContextProps> = { state: {}, api: { getGlobals: fn(() => ({ vision: undefined })), updateGlobals: fn(), getStoryGlobals: fn(() => ({ vision: undefined })), getUserGlobals: fn(() => ({ vision: undefined })), } as any, // api type is complex, cast if needed };Option 2: Reset mocks in each story's play function (if test isolation is needed)
export const Selection = meta.story({ play: async (context) => { // Reset mocks to prevent interference from other stories managerContext.api.updateGlobals.mockClear(); await openMenu(context); await context.userEvent.click(await screen.findByText('Blurred vision')); await expect(managerContext.api.updateGlobals).toHaveBeenCalledWith({ vision: 'blurred' }); // ... }, });
23-23: Improve type safety for Story parameter.The
Storyparameter is typed asany. Consider using a more specific type, such asReact.ComponentTypeor the appropriate Storybook decorator type.🔎 Proposed fix
decorators: [ - (Story: any) => ( + (Story: React.ComponentType) => ( <ManagerContext.Provider value={managerContext}> <Story /> </ManagerContext.Provider> ), ],
48-57: Usecontext.canvasinstead ofscreenfor consistency.Line 51 uses
screen.findByText, while theopenMenufunction (line 34) and the final assertion (lines 54-55) usecontext.canvas. Usingscreencan query elements outside the component's render scope, leading to false positives or interference from other DOM elements.For consistency and proper test scoping, use
context.canvas.findByText.🔎 Proposed fix
play: async (context) => { await openMenu(context); - await context.userEvent.click(await screen.findByText('Blurred vision')); + await context.userEvent.click(await context.canvas.findByText('Blurred vision')); await expect(managerContext.api.updateGlobals).toHaveBeenCalledWith({ vision: 'blurred' }); await expect( context.canvas.getByRole('button', { name: 'Vision simulator Blurred vision' }) ).toBeVisible(); },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.test.tsx
💤 Files with no reviewable changes (1)
- code/addons/a11y/src/components/VisionSimulator.test.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsx
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsx
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/a11y/src/components/VisionSimulator.stories.tsx
🧠 Learnings (6)
📓 Common learnings
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.
📚 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/addons/a11y/src/components/VisionSimulator.stories.tsx
📚 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/addons/a11y/src/components/VisionSimulator.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/addons/a11y/src/components/VisionSimulator.stories.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/addons/a11y/src/components/VisionSimulator.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/addons/a11y/src/components/VisionSimulator.stories.tsx
🧬 Code graph analysis (1)
code/addons/a11y/src/components/VisionSimulator.stories.tsx (3)
code/addons/a11y/src/components/VisionSimulator.tsx (1)
VisionSimulator(37-62)code/core/src/csf/csf-factories.ts (1)
Story(122-154)code/core/src/test/testing-library.ts (1)
userEvent(123-126)
⏰ 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)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/addons/a11y/src/components/VisionSimulator.stories.tsx (1)
6-6: The relative import path works for the a11y addon, as evidenced by consistent usage in multiple story files (VisionSimulator, TestDiscrepancyMessage, A11YPanel, Report). However, consider using subpath imports (e.g.,import preview from '#.storybook/preview') instead, which aligns with modern Storybook patterns and eliminates dependency on the relative path depth.⛔ Skipped due to learnings
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.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: 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.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.
ghengeveld
force-pushed
the
vision-simulator-global
branch
from
b17cdb0 to
3830aed
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
code/addons/a11y/src/withVisionSimulator.ts (1)
42-49: Past review issue partially addressed: duplicate insertion still possible.The cleanup now safely uses optional chaining (good!), but the insertion on line 43 still doesn't check for an existing element. If multiple stories mount simultaneously (e.g., in docs mode), duplicate
<svg>elements with the same ID could be inserted.🔎 Add existence check before insertion
useEffect(() => { - document.body.insertAdjacentHTML('beforeend', filterDefs); + const existingFilters = document.getElementById('storybook-a11y-vision-filters'); + if (!existingFilters) { + document.body.insertAdjacentHTML('beforeend', filterDefs); + } return () => { const filterDefsElement = document.getElementById('storybook-a11y-vision-filters'); filterDefsElement?.parentElement?.removeChild(filterDefsElement); }; }, []);
🧹 Nitpick comments (1)
code/addons/a11y/src/withVisionSimulator.ts (1)
7-8: Consider the regex construction approach.The regex is built from filter strings that contain special characters like parentheses, quotes, and hashes. Word boundaries (
\b) may not behave as expected with these characters in all contexts (e.g.,\bafter)might not match correctly).Since the filters come from a trusted local module, the static analysis ReDoS warning is likely a false positive. However, if you encounter issues with filter removal, consider escaping special regex characters or using a simpler string-based approach.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/a11y/src/withVisionSimulator.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/a11y/src/withVisionSimulator.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/a11y/src/withVisionSimulator.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/a11y/src/withVisionSimulator.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/a11y/src/withVisionSimulator.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/a11y/src/withVisionSimulator.ts
🧠 Learnings (6)
📓 Common learnings
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.
📚 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/addons/a11y/src/withVisionSimulator.ts
📚 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/addons/a11y/src/withVisionSimulator.ts
📚 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/addons/a11y/src/withVisionSimulator.ts
📚 Learning: 2025-11-05T09:36:55.944Z
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.
Applied to files:
code/addons/a11y/src/withVisionSimulator.ts
📚 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/addons/a11y/src/withVisionSimulator.ts
🧬 Code graph analysis (1)
code/addons/a11y/src/withVisionSimulator.ts (2)
code/addons/a11y/src/visionSimulatorFilters.ts (2)
filters(1-46)filterDefs(48-100)code/frameworks/angular/src/client/public-types.ts (1)
StoryFn(35-38)
🪛 ast-grep (0.40.3)
code/addons/a11y/src/withVisionSimulator.ts
[warning] 7-7: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b(${knownFilters.join('|')})\\b, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @code/addons/a11y/src/components/A11YPanel.test.tsx:
- Line 2: The triple-slash TypeScript directive at the top of the test file is
terminated with an extra semicolon which makes the directive invalid; remove the
trailing semicolon from the triple-slash reference directive (the line starting
with /// <reference types="@testing-library/jest-dom">) so it becomes a proper
TS triple-slash directive without a semicolon.
🧹 Nitpick comments (1)
code/addons/a11y/src/components/A11YPanel.test.tsx (1)
15-23: Align mocking patterns with coding guidelines.The current mocking setup has two deviations from the project's testing guidelines:
- Lines 15 and 18:
vi.mock()calls are missing thespy: trueoption- Lines 21-23: Mock implementation is outside a
beforeEachblockAs per coding guidelines, all mocks should use
spy: trueand mock behaviors should be implemented inbeforeEachblocks for consistency and proper cleanup between tests.♻️ Proposed refactor
-vi.mock('storybook/manager-api'); +vi.mock('storybook/manager-api', { spy: true }); const mockedManagerApi = vi.mocked(managerApi); -vi.mock('./A11yContext'); +vi.mock('./A11yContext', { spy: true }); const mockedUseA11yContext = vi.mocked(useA11yContext); -mockedManagerApi.useParameter.mockReturnValue({ - manual: false, -} as any);Then add a
beforeEachblock before the test suite:describe('A11YPanel', () => { beforeEach(() => { mockedManagerApi.useParameter.mockReturnValue({ manual: false, } as any); }); // ... rest of tests });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/a11y/src/components/A11YPanel.test.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/addons/a11y/src/components/A11YPanel.test.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/addons/a11y/src/components/A11YPanel.test.tsx
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/a11y/src/components/A11YPanel.test.tsx
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/a11y/src/components/A11YPanel.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/a11y/src/components/A11YPanel.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/a11y/src/components/A11YPanel.test.tsx
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Usevi.mock()to mock file system, loggers, and other external dependencies in tests
Files:
code/addons/a11y/src/components/A11YPanel.test.tsx
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/a11y/src/components/A11YPanel.test.tsx
🧠 Learnings (17)
📓 Common learnings
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.
📚 Learning: 2025-11-05T09:36:55.944Z
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.
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-10-03T07:55:42.639Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/manager/components/preview/Toolbar.tsx:102-105
Timestamp: 2025-10-03T07:55:42.639Z
Learning: In code/core/src/manager/components/preview/Toolbar.tsx, we intentionally do not add aria-label/aria-labelledby to StyledToolbar (AbstractToolbar) because the enclosing section is already labeled via an sr-only heading and the toolbar is the only content. Revisit only if real user testing indicates a need.
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mocked()` to type and access the mocked functions in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with `vi.mocked()` in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
Applied to files:
code/addons/a11y/src/components/A11YPanel.test.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). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
There was a problem hiding this comment.
Amazing work @ghengeveld this is an fantastic improvement!
I was going to suggest we add stories for this new behavior, but then I noticed that you did 👏
Merge it!
ghengeveld
added
the
needs qa
ndelangen
removed
the
needs qa
2 participants
What I did
Synchronize vision simulator state with globals and apply vision filters in the preview rather than on the manager's preview iframe. This allows stories to specify a vision deficiency via
globalsand have Chromatic snapshot it as such.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
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
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.