Storybook investigation#145
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 16 minutes and 13 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces Storybook 10.4.1 with a comprehensive library of 50+ export format preview stories (covering code languages like JavaScript, Java, Python, and unit test frameworks like Jest, PyTest, JUnit), interactive story harnesses for test-data and grid-preview UI surfaces, and a GitHub Pages deployment pipeline for publishing a static test environment. ChangesStorybook Framework & Configuration
Export Format Story Library
Story Harnesses & Testing
Test Environment Build & Deployment
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a Storybook-based investigation environment for UI components, along with a script and GitHub Pages workflow that publishes a combined static "test environment" (app, generator, Storybook) for review. Adds Storybook configuration, a large set of stories for the Test Data panels and export-format previews, and a Jest test suite covering the new story harnesses.
Changes:
- Add Storybook 10.4.1 setup (
.storybook/, dependencies,pnpm run storybook/build-storybookscripts) with stories for Test Data (embedded panel, generator) and many Export Formats (CSV, JSON, code/unit-test variants, etc.) plus shared harnesses and interaction plays. - Add
scripts/create-testenv.mjsand.github/workflows/publish-testenv.ymlto build and deploy a static testenv bundle (app, generator, Storybook) to GitHub Pages, plus README guidance and .gitignore updates. - Add Jest coverage for storybook harnesses and a small CSS path tweak (
/images/...→./images/...) to support Storybook serving.
Reviewed changes
Copilot reviewed 60 out of 62 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/create-testenv.mjs | New script orchestrating vite + storybook builds and rendering an index page. |
| .github/workflows/publish-testenv.yml | New workflow that builds testenv and deploys to GitHub Pages on push to master. |
| .storybook/main.js, .storybook/preview.js, .storybook/package.json | Storybook config (html-vite framework, aliases, addons). |
| README.md | Documents Storybook usage and story groups. |
| package.json, pnpm-lock.yaml, pnpm-workspace.yaml | Add Storybook deps and esbuild to allowBuilds. |
| apps/web/src/stories/*.stories.js | Story files for Test Data panels and many export formats. |
| apps/web/src/stories/storybook-harnesses.js | Core harness that mounts app fragments and emits Storybook actions. |
| apps/web/src/stories/export-format-story-factory.js, export-format-interactions.js | Shared story factory and Storybook play helpers. |
| apps/web/src/stories/package.json | Marks the stories folder as ES modules. |
| apps/web/src/tests/jest/storybook/storybook-harnesses.test.js | Jest coverage for the harnesses. |
| apps/web/styles.css | Switches helpicon background image to a relative URL for Storybook compatibility. |
| .gitignore | Ignores storybook-static/ and testenv/ outputs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46f293b465
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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 (3)
apps/web/src/stories/export-format-code-unit-test-jest.stories.js (1)
8-35: ⚡ Quick winConsider extending documentation pattern to other unit test stories.
The comprehensive documentation and
playCodePreviewinteraction in this Jest story provide excellent context and interactivity. The other unit test framework stories (JUnit, Mocha, Vitest, etc.) currently lack this documentation and interactive demo, which creates an inconsistent user experience across the story library.Consider applying a similar documentation pattern to the other unit test story files to improve discoverability and consistency across the Storybook UI.
Also applies to: 40-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/stories/export-format-code-unit-test-jest.stories.js` around lines 8 - 35, The Jest story's docs block (parameters.docs.description.component) and its interactive play handler (playCodePreview) provide rich documentation and demo behavior that should be replicated across the other unit-test stories (e.g., the JUnit, Mocha, Vitest story modules that define "Start Blank" and "Previewed" stories). For each unit-test story file, copy the descriptive markdown from parameters.docs.description.component and wire the same playCodePreview interaction into the story exports so the preview area and Actions demo behave identically; ensure story names ("Start Blank", "Previewed") and any controls/Set Text From Grid button selectors used by playCodePreview are consistent or adapted per file. Replace only the docs string and play handler wiring—do not change the story logic—and run Storybook to verify consistent documentation and interactivity across all unit-test format stories.apps/web/src/stories/export-format-code-unit-test-python-pytest.stories.js (1)
4-4: 💤 Low valueConsider using official "pytest" capitalization.
The official framework name is "pytest" (lowercase), not "PyTest". For consistency with the project's branding, consider updating the title to match.
♻️ Suggested capitalization fix
- title: 'Export Formats/Previews/Code (Unit Test)/Python/PyTest', + title: 'Export Formats/Previews/Code (Unit Test)/Python/pytest',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/stories/export-format-code-unit-test-python-pytest.stories.js` at line 4, The story title string currently uses "PyTest" which is inconsistent with the official lowercase branding; update the title value in the file (the string passed to title, e.g. "Export Formats/Previews/Code (Unit Test)/Python/PyTest") to use "pytest" instead (so it reads "Export Formats/Previews/Code (Unit Test)/Python/pytest") to maintain consistent capitalization across stories..github/workflows/publish-testenv.yml (1)
8-11: ⚡ Quick winScope permissions to job level for least privilege.
Currently,
pages: writeandid-token: writeare granted at the workflow level, but only thedeployjob requires them. Thebuildjob only needscontents: read. Following the principle of least privilege, move these permissions to their respective jobs:🔐 Recommended permission scoping
-permissions: - contents: read - pages: write - id-token: write - concurrency: group: pages cancel-in-progress: true jobs: build: name: Build testenv runs-on: ubuntu-latest + permissions: + contents: read env:deploy: name: Deploy to GitHub Pages needs: build runs-on: ubuntu-latest + permissions: + pages: write + id-token: write environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }}As per coding guidelines, this aligns with GitHub Actions security best practices for limiting blast radius of compromised jobs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-testenv.yml around lines 8 - 11, The workflow-level permissions block currently grants permissions: contents: read, pages: write, id-token: write; restrict these by moving pages: write and id-token: write into the deploy job's permissions and ensure the build job only has contents: read (or move workflow-level permissions to none/minimal), i.e., update the jobs named "build" and "deploy" so "build" has permissions: contents: read and "deploy" has permissions: pages: write and id-token: write (add contents: read to deploy if needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-testenv.yml:
- Around line 27-28: The checkout step ("Check out repository" using
actions/checkout@v4) is leaving credentials persisted and could leak when the
testenv artifact is uploaded; update that step to disable credential persistence
by adding persist-credentials: false (and optionally set fetch-depth: 0 if full
history is needed) so the runner won't retain repository credentials before the
artifact upload.
In `@apps/web/src/stories/storybook-harnesses.js`:
- Around line 113-136: withScopedDocument currently restores document overrides
immediately because it returns callback() inside a try/finally; change it to
capture callback() result, detect if it's a Promise (has a then), and if so
return result.finally(() => { restore document.querySelector, querySelectorAll,
getElementById to originalQuerySelector, originalQuerySelectorAll,
originalGetElementById }); otherwise restore synchronously and return the value.
Update the implementation around the callback invocation in withScopedDocument
to use this promise-aware restore logic so async callbacks (e.g.,
importExportController.importTextArea / _previewThenImportToGrid) see the scoped
document for the full async duration.
---
Nitpick comments:
In @.github/workflows/publish-testenv.yml:
- Around line 8-11: The workflow-level permissions block currently grants
permissions: contents: read, pages: write, id-token: write; restrict these by
moving pages: write and id-token: write into the deploy job's permissions and
ensure the build job only has contents: read (or move workflow-level permissions
to none/minimal), i.e., update the jobs named "build" and "deploy" so "build"
has permissions: contents: read and "deploy" has permissions: pages: write and
id-token: write (add contents: read to deploy if needed).
In `@apps/web/src/stories/export-format-code-unit-test-jest.stories.js`:
- Around line 8-35: The Jest story's docs block
(parameters.docs.description.component) and its interactive play handler
(playCodePreview) provide rich documentation and demo behavior that should be
replicated across the other unit-test stories (e.g., the JUnit, Mocha, Vitest
story modules that define "Start Blank" and "Previewed" stories). For each
unit-test story file, copy the descriptive markdown from
parameters.docs.description.component and wire the same playCodePreview
interaction into the story exports so the preview area and Actions demo behave
identically; ensure story names ("Start Blank", "Previewed") and any
controls/Set Text From Grid button selectors used by playCodePreview are
consistent or adapted per file. Replace only the docs string and play handler
wiring—do not change the story logic—and run Storybook to verify consistent
documentation and interactivity across all unit-test format stories.
In `@apps/web/src/stories/export-format-code-unit-test-python-pytest.stories.js`:
- Line 4: The story title string currently uses "PyTest" which is inconsistent
with the official lowercase branding; update the title value in the file (the
string passed to title, e.g. "Export Formats/Previews/Code (Unit
Test)/Python/PyTest") to use "pytest" instead (so it reads "Export
Formats/Previews/Code (Unit Test)/Python/pytest") to maintain consistent
capitalization across stories.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 898636b2-58bf-460e-be7d-0841e74b67fc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (61)
.github/workflows/publish-testenv.yml.gitignore.storybook/main.js.storybook/package.json.storybook/preview.jsREADME.mdapps/web/src/stories/export-format-ascii.stories.jsapps/web/src/stories/export-format-code-csharp.stories.jsapps/web/src/stories/export-format-code-java.stories.jsapps/web/src/stories/export-format-code-javascript.stories.jsapps/web/src/stories/export-format-code-kotlin.stories.jsapps/web/src/stories/export-format-code-perl.stories.jsapps/web/src/stories/export-format-code-php.stories.jsapps/web/src/stories/export-format-code-python.stories.jsapps/web/src/stories/export-format-code-ruby.stories.jsapps/web/src/stories/export-format-code-typescript.stories.jsapps/web/src/stories/export-format-code-unit-test-csharp-mstest.stories.jsapps/web/src/stories/export-format-code-unit-test-csharp-nunit.stories.jsapps/web/src/stories/export-format-code-unit-test-csharp-xunit.stories.jsapps/web/src/stories/export-format-code-unit-test-java-junit4.stories.jsapps/web/src/stories/export-format-code-unit-test-java-junit5.stories.jsapps/web/src/stories/export-format-code-unit-test-java-junit6.stories.jsapps/web/src/stories/export-format-code-unit-test-java-testng.stories.jsapps/web/src/stories/export-format-code-unit-test-javascript-mocha.stories.jsapps/web/src/stories/export-format-code-unit-test-javascript-vitest.stories.jsapps/web/src/stories/export-format-code-unit-test-jest.stories.jsapps/web/src/stories/export-format-code-unit-test-kotlin-junit5-kotlin.stories.jsapps/web/src/stories/export-format-code-unit-test-kotlin-kotest.stories.jsapps/web/src/stories/export-format-code-unit-test-kotlin-spek.stories.jsapps/web/src/stories/export-format-code-unit-test-perl-test-more.stories.jsapps/web/src/stories/export-format-code-unit-test-perl-test2-suite.stories.jsapps/web/src/stories/export-format-code-unit-test-php-pest.stories.jsapps/web/src/stories/export-format-code-unit-test-php-phpunit.stories.jsapps/web/src/stories/export-format-code-unit-test-python-nose2.stories.jsapps/web/src/stories/export-format-code-unit-test-python-pytest.stories.jsapps/web/src/stories/export-format-code-unit-test-python-unittest.stories.jsapps/web/src/stories/export-format-code-unit-test-ruby-minitest.stories.jsapps/web/src/stories/export-format-code-unit-test-ruby-rspec.stories.jsapps/web/src/stories/export-format-code-unit-test-typescript-jest.stories.jsapps/web/src/stories/export-format-code-unit-test-typescript-mocha.stories.jsapps/web/src/stories/export-format-code-unit-test-typescript-vitest.stories.jsapps/web/src/stories/export-format-csv.stories.jsapps/web/src/stories/export-format-delimited.stories.jsapps/web/src/stories/export-format-gherkin.stories.jsapps/web/src/stories/export-format-html.stories.jsapps/web/src/stories/export-format-interactions.jsapps/web/src/stories/export-format-json.stories.jsapps/web/src/stories/export-format-jsonl.stories.jsapps/web/src/stories/export-format-markdown.stories.jsapps/web/src/stories/export-format-sql.stories.jsapps/web/src/stories/export-format-story-factory.jsapps/web/src/stories/export-format-xml.stories.jsapps/web/src/stories/package.jsonapps/web/src/stories/storybook-harnesses.jsapps/web/src/stories/test-data-embedded-panel.stories.jsapps/web/src/stories/test-data-generator.stories.jsapps/web/src/tests/jest/storybook/storybook-harnesses.test.jsapps/web/styles.csspackage.jsonpnpm-workspace.yamlscripts/create-testenv.mjs
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
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.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores