Add storybook for help panel#248
Closed
apinkert wants to merge 9 commits intoRedHatInsights:masterfrom
Closed
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Storybook integration: configuration and preview, Storybook scripts and devDependencies, mock hooks for Chrome/RBAC/Unleash, MSW service worker and loader setup, multiple documentation MDX pages, and an ESLint rule tweak to ignore underscore-prefixed unused args. Changes
Sequence Diagram(s)sequenceDiagram
participant Story as Storybook
participant Decorators as Decorators (Intl/Providers)
participant App as App Component / Hooks
participant MSW as MSW Service Worker
participant Network as Network
rect rgba(135,206,250,0.5)
Story->>Decorators: Render story with decorators
Decorators->>App: Provide Intl, FlagProvider, RBAC, Chrome mocks
end
rect rgba(144,238,144,0.5)
App->>Network: fetch(...) (API request)
Network->>MSW: intercepted by service worker
MSW->>App: postMessage serialized request to story client
App->>MSW: respond with MOCK_RESPONSE or PASSTHROUGH
MSW->>Network: return mocked or passthrough response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
6-7:⚠️ Potential issue | 🟠 MajorRaise Node engine floor to >=20.0.0 to match
@storybook/react-webpack5requirements.The
@storybook/react-webpack5@9.1.19package requires Node >=20.0.0, but the current constraint allows Node >=16.20.2. Users with Node 16–19 would encounter failures duringstorybook,storybook dev,build-storybook, andtest-storybookcommands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 6 - 7, Update the Node engine constraint in package.json so Storybook's requirement is met: change the engines.node field (in package.json) from ">=16.20.2" to ">=20.0.0" and verify any CI configs or README tooling notes reflect the new minimum Node version; keep engines.npm as-is unless you also need to raise npm, and run a quick CI/local Storybook start to confirm compatibility.
🧹 Nitpick comments (3)
.storybook/hooks/useChrome.tsx (1)
3-39: Consider a silent-by-default logger in this mock.Frequent
console.logcalls can add noise in Storybook and test runs; a debug toggle keeps this useful without default verbosity.Proposed refactor
+const debugMocks = Boolean((globalThis as { __STORYBOOK_DEBUG_MOCKS__?: boolean }).__STORYBOOK_DEBUG_MOCKS__); +const log = (...args: unknown[]) => { + if (debugMocks) console.log(...args); +}; + export const useChrome = () => ({ analytics: { track: (event: string, properties?: Record<string, unknown>) => { - console.log('Analytics track:', event, properties); + log('Analytics track:', event, properties); }, }, @@ helpTopics: { setActiveTopic: (topic: string) => { - console.log('Setting active help topic:', topic); + log('Setting active help topic:', topic); }, enableTopics: (topics: string[]) => { - console.log('Enabling help topics:', topics); + log('Enabling help topics:', topics); }, disableTopics: (topics: string[]) => { - console.log('Disabling help topics:', topics); + log('Disabling help topics:', topics); }, }, quickStarts: { set: (quickStarts: unknown[]) => { - console.log('Setting quick starts:', quickStarts); + log('Setting quick starts:', quickStarts); }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.storybook/hooks/useChrome.tsx around lines 3 - 39, Replace noisy console.log calls in the Storybook mock with a silent-by-default logger that only emits when a debug flag is set; create a simple logger object (e.g., debugLogger with methods debug/info/warn) and a toggle (e.g., DEBUG_MOCKS or logger.enabled) and update the mock functions—analytics.track, auth.getUser (if it needs logging), helpTopics.setActiveTopic, helpTopics.enableTopics, helpTopics.disableTopics, and quickStarts.set—to call debugLogger.debug(...) instead of console.log so logs appear only when the debug toggle is enabled while preserving the same messages and signatures..storybook/hooks/RBACHook.ts (1)
2-6: Make RBAC mock state overridable to support denied/loading stories.This hardcoded return is fine for a default, but it limits scenario testing in Storybook. Consider exposing a lightweight setter for story-level overrides.
Proposed refactor
+type MockPermissionsState = { + hasAccess: boolean; + isLoading: boolean; + permissions: unknown[]; +}; + +let mockPermissionsState: MockPermissionsState = { + hasAccess: true, + isLoading: false, + permissions: [], +}; + +export const __setMockPermissionsState = ( + next: Partial<MockPermissionsState> +) => { + mockPermissionsState = { ...mockPermissionsState, ...next }; +}; + export const usePermissions = () => ({ - hasAccess: true, - isLoading: false, - permissions: [], + ...mockPermissionsState, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.storybook/hooks/RBACHook.ts around lines 2 - 6, Replace the hardcoded usePermissions return with an overridable mock: change the exported function usePermissions to accept an optional override object (or initialize internal state) and return both the current permissions state and a setter (e.g., setPermissions) so stories can simulate denied/loading states; update any callers to use the new tuple/object shape (hasAccess, isLoading, permissions, setPermissions) so Storybook stories can call setPermissions({ hasAccess: false }) or setPermissions({ isLoading: true }) to override the defaults..storybook/preview.tsx (1)
26-29: Callinitialize()at module scope instead ofbeforeAll.This aligns with the canonical pattern shown in official Storybook documentation for
msw-storybook-addonsetup. WhilebeforeAllis officially supported,initialize()is typically called once at module scope before thePreviewobject definition for clarity and consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.storybook/preview.tsx around lines 26 - 29, The initialize({ onUnhandledRequest: 'warn' }) call should be moved out of the beforeAll property and invoked at module scope before the Preview object is defined; remove the beforeAll entry from the exported Preview configuration and keep loaders: [mswLoader] intact. Locate the existing beforeAll block that calls initialize and instead call initialize(...) once at top-level of .storybook/preview.tsx (above where Preview or export const parameters/export default is declared), preserving the onUnhandledRequest option so msw-storybook-addon is initialized exactly once at module load.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.storybook/hooks/unleash.ts:
- Around line 2-11: The Storybook hook currently hardcodes all flags as false,
causing feature-gated UI to be hidden; update useFlags to return a configurable
default map (not all false) and update useFlag(flag) to return the lookup from
that map (i.e., return useFlags()[flag]) so stories render realistic states;
also allow overrides via a simple mechanism (e.g., read a global/window override
like window.__UNLEASH_FLAGS__ or Storybook globals/env var) inside useFlags so
individual stories or QA can flip flags without changing code; change
references: useFlag and useFlags.
In @.storybook/main.ts:
- Around line 7-8: The MDX glob '../src/docs/*.mdx' overlaps with and is
subsumed by '../src/**/*.mdx', causing duplicate Storybook entries; remove the
specific '../src/docs/*.mdx' entry from the stories array (leave
'../src/**/*.mdx' intact) so only the recursive glob '../src/**/*.mdx' remains
and duplicates are eliminated.
---
Outside diff comments:
In `@package.json`:
- Around line 6-7: Update the Node engine constraint in package.json so
Storybook's requirement is met: change the engines.node field (in package.json)
from ">=16.20.2" to ">=20.0.0" and verify any CI configs or README tooling notes
reflect the new minimum Node version; keep engines.npm as-is unless you also
need to raise npm, and run a quick CI/local Storybook start to confirm
compatibility.
---
Nitpick comments:
In @.storybook/hooks/RBACHook.ts:
- Around line 2-6: Replace the hardcoded usePermissions return with an
overridable mock: change the exported function usePermissions to accept an
optional override object (or initialize internal state) and return both the
current permissions state and a setter (e.g., setPermissions) so stories can
simulate denied/loading states; update any callers to use the new tuple/object
shape (hasAccess, isLoading, permissions, setPermissions) so Storybook stories
can call setPermissions({ hasAccess: false }) or setPermissions({ isLoading:
true }) to override the defaults.
In @.storybook/hooks/useChrome.tsx:
- Around line 3-39: Replace noisy console.log calls in the Storybook mock with a
silent-by-default logger that only emits when a debug flag is set; create a
simple logger object (e.g., debugLogger with methods debug/info/warn) and a
toggle (e.g., DEBUG_MOCKS or logger.enabled) and update the mock
functions—analytics.track, auth.getUser (if it needs logging),
helpTopics.setActiveTopic, helpTopics.enableTopics, helpTopics.disableTopics,
and quickStarts.set—to call debugLogger.debug(...) instead of console.log so
logs appear only when the debug toggle is enabled while preserving the same
messages and signatures.
In @.storybook/preview.tsx:
- Around line 26-29: The initialize({ onUnhandledRequest: 'warn' }) call should
be moved out of the beforeAll property and invoked at module scope before the
Preview object is defined; remove the beforeAll entry from the exported Preview
configuration and keep loaders: [mswLoader] intact. Locate the existing
beforeAll block that calls initialize and instead call initialize(...) once at
top-level of .storybook/preview.tsx (above where Preview or export const
parameters/export default is declared), preserving the onUnhandledRequest option
so msw-storybook-addon is initialized exactly once at module load.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.eslintrc.js.storybook/hooks/RBACHook.ts.storybook/hooks/unleash.ts.storybook/hooks/useChrome.tsx.storybook/main.ts.storybook/preview.tsxpackage.jsonsrc/docs/Architecture.mdxsrc/docs/Introduction.mdxsrc/docs/QuickReference.mdxsrc/docs/StorybookGuide.mdxstatic/mockServiceWorker.js
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
package.json (1)
24-25:test-storybook:ciis not self-contained unless Storybook is started elsewhere.
test-storybookexpects a reachable Storybook instance (default:6006). If CI does not start/serve Storybook before this script, it will fail. Either make the script orchestration explicit or document/enforce the precondition.Reference:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 24 - 25, The CI script test-storybook:ci currently runs test-storybook against a live Storybook but doesn't start Storybook itself; update package.json so test-storybook:ci is self-contained by either (A) composing a new script that starts Storybook in CI mode, waits for it to be reachable, runs test-storybook --ci --excludeTags test-skip, and then stops the Storybook process, or (B) add a precondition note and a pretest script (e.g., pretest-storybook) that launches Storybook before test-storybook:ci; update or add scripts referencing the existing "test-storybook" and "test-storybook:ci" entries so CI always serves Storybook before running tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 61: Update the Storybook test runner dependency in package.json from
"@storybook/test-runner": "^0.23.0" to the compatible "^0.19.0" (or the latest
0.19.x) to match Storybook ^9.1.19; after changing the version string for
"@storybook/test-runner" run your package manager (npm install / yarn install /
pnpm install) to update node_modules and the lockfile, and then run the
Storybook tests to confirm everything works with the Test Runner change.
- Around line 6-7: Update the engine constraints in package.json so they meet
Storybook 9 requirements by changing the "node" and "npm" engine entries: set
"node" to ">=20.0.0" and "npm" to ">=10.0.0" (modify the existing "node" and
"npm" keys in package.json accordingly) to ensure installs and runtime match
Storybook 9's minimums.
---
Nitpick comments:
In `@package.json`:
- Around line 24-25: The CI script test-storybook:ci currently runs
test-storybook against a live Storybook but doesn't start Storybook itself;
update package.json so test-storybook:ci is self-contained by either (A)
composing a new script that starts Storybook in CI mode, waits for it to be
reachable, runs test-storybook --ci --excludeTags test-skip, and then stops the
Storybook process, or (B) add a precondition note and a pretest script (e.g.,
pretest-storybook) that launches Storybook before test-storybook:ci; update or
add scripts referencing the existing "test-storybook" and "test-storybook:ci"
entries so CI always serves Storybook before running tests.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For RHCLOUD-45241
This PR just adds the basic npm framework for storybook and the overview/arch docs for the help panel that can be viewed via
npm run storybook