Controls: Improve ArgsTable empty state guidance#34857
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:
📝 WalkthroughWalkthroughEmptyTabContent copy now uses the title "This story has no controls" with guidance to define ChangesEmpty State UI Update and Testing
🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/docs/src/blocks/components/ArgsTable/Empty.test.tsx (1)
1-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove these assertions into stories and remove this component unit test file.
Line 1 introduces a
*.test.tsxfor a React component, but this repo requires interaction/behavior assertions to live in*.stories.tsxplay functions (which this PR already adds inArgsTable.stories.tsx). Keeping both creates policy drift and redundant maintenance.As per coding guidelines, “For React components, write Storybook stories with
playfunctions — do NOT write*.test.tsxunit tests; behavior, accessibility, and interaction assertions belong in*.stories.tsxco-located with the component”.🤖 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 `@code/addons/docs/src/blocks/components/ArgsTable/Empty.test.tsx` around lines 1 - 33, Remove the unit test file Empty.test.tsx and move its interaction/assertion logic into the story play function(s) for the Empty component in ArgsTable.stories.tsx; specifically, take the assertions that use renderEmpty, screen.findByText, screen.queryByText, and screen.getByRole (the checks for "No controls available for this story", the guidance text, and the "Learn how to configure controls" link href) and implement them as play-time assertions in the appropriate story's play() so behavior testing lives in the story, then delete the Empty.test.tsx file to avoid duplicate tests.
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/components/ArgsTable/Empty.test.tsx (1)
9-9: ⚡ Quick winUse explicit extension for the relative import.
Line 9 should use an explicit module extension for consistency with repo TypeScript import rules.
Proposed fix
-import { Empty } from './Empty'; +import { Empty } from './Empty.tsx';As per coding guidelines, “For TypeScript source in the repo, prefer explicit file extensions for relative code imports and exports such as
./foo.tsor./bar.tsxwhen the target is another TS/JS module”.🤖 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 `@code/addons/docs/src/blocks/components/ArgsTable/Empty.test.tsx` at line 9, Update the relative import to include the explicit TypeScript extension: replace the import of the Empty component ("import { Empty } from './Empty';") with the explicit extension form ("import { Empty } from './Empty.tsx';") so it follows the repo rule for TypeScript source imports and resolves the Empty symbol correctly.
🤖 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.
Outside diff comments:
In `@code/addons/docs/src/blocks/components/ArgsTable/Empty.test.tsx`:
- Around line 1-33: Remove the unit test file Empty.test.tsx and move its
interaction/assertion logic into the story play function(s) for the Empty
component in ArgsTable.stories.tsx; specifically, take the assertions that use
renderEmpty, screen.findByText, screen.queryByText, and screen.getByRole (the
checks for "No controls available for this story", the guidance text, and the
"Learn how to configure controls" link href) and implement them as play-time
assertions in the appropriate story's play() so behavior testing lives in the
story, then delete the Empty.test.tsx file to avoid duplicate tests.
---
Nitpick comments:
In `@code/addons/docs/src/blocks/components/ArgsTable/Empty.test.tsx`:
- Line 9: Update the relative import to include the explicit TypeScript
extension: replace the import of the Empty component ("import { Empty } from
'./Empty';") with the explicit extension form ("import { Empty } from
'./Empty.tsx';") so it follows the repo rule for TypeScript source imports and
resolves the Empty symbol correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc2fdca1-daa1-4584-ab35-e991923cfed3
📒 Files selected for processing (3)
code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.stories.tsxcode/addons/docs/src/blocks/components/ArgsTable/Empty.test.tsxcode/addons/docs/src/blocks/components/ArgsTable/Empty.tsx
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
Sidnioulz
left a comment
Sidnioulz
left a comment
There was a problem hiding this comment.
I find myself in stark approval! Thank you for this PR!
I will run this by our design team for content review and will make sure it gets merged. I'll let you know if you have any followup actions.
Sidnioulz
changed the title
Sidnioulz
force-pushed
the
docs/args-table-empty-state-guidance
branch
from
4e465fd to
3c250b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.stories.tsx`:
- Line 163: Delete the pre-delay negative assertion in the story play: remove
the line that does await expect(canvas.queryByText('This story has no
controls')).not.toBeInTheDocument(); in ArgsTable.stories.tsx because Empty.tsx
intentionally returns null then sets isLoading after a timeout and that negative
check makes the test flaky; target the play block in the ArgsTable story (where
canvas.queryByText is used) and either omit the negative assertion entirely or
replace it with a deterministic wait/assert for the final state instead.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7b307c5-7ebe-4a0d-8174-aeb9f1e6f0ae
📒 Files selected for processing (2)
code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.stories.tsxcode/addons/docs/src/blocks/components/ArgsTable/Empty.tsx
github-project-automation
Bot
moved this from Empathy Queue (prioritized)
to Done
in Core Team Projects
3 participants
What I did
Improved the ArgsTable empty state in addon-docs to make the "no controls" case more actionable.
args,argTypes, and docgen affect controlsLearn how to configure controlsChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Components/ArgsTable/ArgsTablestoriesEmptyEmptyInsideAddonPanelNo controls available for this storyargs,argTypes, and docgenLearn how to configure controlsDocumentation
Summary by CodeRabbit