frontend: daemonset/List: add missing Storybook states#5698
frontend: daemonset/List: add missing Storybook states#5698sudhidutta7694 wants to merge 1 commit into
Conversation
|
|
|
Welcome @sudhidutta7694! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sudhidutta7694 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
illume
left a comment
There was a problem hiding this comment.
Thanks for working on this.
The commit messages could use some tidying up to match our contribution guidelines. We use Linux kernel style — the contributing guide has the details, and git log shows good examples.
Commits that need attention
frontend: daemonset/List: add missing Storybook states— Description must start with a capital letter — e.g.frontend: HomeButton: Fix the buttonnotfrontend: HomeButton: fix the button.
Commit guidelines
- Use atomic commits focused on a single change.
- Use the title format
<area>: <Description of changes>— description must start with a capital letter. - Keep the title under 72 characters (soft requirement).
- Explain the intention and why the change is needed.
- Make commit titles meaningful and describe what changed.
- Do not add code that a later commit rewrites; squash or reorder commits instead.
- Do not include
Fixes #NNin commit messages.
Good examples:
frontend: HomeButton: Fix so it navigates to homebackend: config: Add enable-dynamic-clusters flag
There was a problem hiding this comment.
Pull request overview
Adds four new Storybook stories (Loading, Empty, Error, LongName) for the DaemonSet List component to cover previously missing visual states, plus the corresponding storyshot snapshots.
Changes:
- Add
Loadingstory using a never-resolving MSW handler (withstoryshots.disable: true). - Add
EmptyandErrorstories with corresponding MSW handlers and snapshots. - Add
LongNamestory to verify long name/namespace overflow handling, with snapshot.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/src/components/daemonset/List.stories.tsx | New Loading, Empty, Error, LongName story exports with MSW handlers |
| frontend/src/components/daemonset/snapshots/List.Empty.stories.storyshot | New snapshot for the Empty state |
| frontend/src/components/daemonset/snapshots/List.Error.stories.storyshot | New snapshot for the Error state |
| frontend/src/components/daemonset/snapshots/List.LongName.stories.storyshot | New snapshot for the LongName state |
illume
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
The GitHub CI test job has snapshot failures. Run cd frontend && npm run test -- -u to regenerate the snapshots.
How to update snapshots
Run cd frontend && npm run test -- -u to regenerate all snapshots. Review the diff to make sure the visual changes are intentional, then commit the updated snapshot files.
c0d5bd2 to
96c892c
Compare
|
Hi @illume, thanks for the feedback! Fixed the commit title capitalisation, rebased onto the latest main, and re-ran the storybook tests. Force-pushed the updated branch. Let me know if there's anything else! |
96c892c to
25ae747
Compare
Add Loading, Empty, Error and LongName stories to the DaemonSet list view, which previously only had the happy-path DaemonSets story. - Loading: MSW handler never resolves, shows spinner indefinitely. Storyshots disabled to prevent test hangs. - Empty: API returns an empty items array, shows 'No data to be shown' - Error: API returns a network-level failure, shows 'Failed to load resources' - LongName: item with a very long name and namespace to verify truncation and overflow behaviour in the table Storyshot snapshots included for Empty, Error and LongName.
25ae747 to
c8683fc
Compare
Summary
Add four missing visual states to the DaemonSet list Storybook stories. The component previously only had a happy-path story (
DaemonSets). This addsLoading,Empty,ErrorandLongNamestates as suggested by @illume on the #headlamp Slack channel.Changes
frontend/src/components/daemonset/List.stories.tsx— added four new exported storiesLoading: MSW handler returns a never-resolving promise so the spinner shows indefinitely.storyshotsdisabled to prevent test hangs.Empty: API returnsitems: [], component shows "No data to be shown"Error: API returns a network-level failure viaHttpResponse.error(), component shows "Failed to load resources"LongName: item with a very long name and namespace to verify truncation and overflow behaviour in the tablefrontend/src/components/daemonset/__snapshots__/— storyshot snapshots added forEmpty,ErrorandLongNameSteps to Test
cd frontend && npm installnpm run storybook -- --no-openNotes for the Reviewer
Loadingstory uses() => new Promise(() => {})— the same pattern ascluster/Overview.stories.tsxLoadingState.Screenshots
Loading

Empty

Error

Long Name
