Skip to content

frontend: cluster: Add Overview stories#3412

Merged
illume merged 1 commit intokubernetes-sigs:mainfrom
1012Charan:add-overview-story-variants
Aug 14, 2025
Merged

frontend: cluster: Add Overview stories#3412
illume merged 1 commit intokubernetes-sigs:mainfrom
1012Charan:add-overview-story-variants

Conversation

@1012Charan
Copy link
Contributor

@1012Charan 1012Charan commented May 31, 2025

Adds 3 Storybook stories for the Overview component:

  • EmptyState (shows the UI when there's no data available)
  • LoadingState (shows a spinner while data is being fetched)
  • ErrorState (simulates when data fails to load)

Screenshots for new stories :
EmptyState
Image 20-06-25 at 3 20 PM
LoadingState
Image 20-06-25 at 3 20 PM (1)
ErrorState
Image 20-06-25 at 3 21 PM

Note: I noticed that the Empty, Loading, and Error states all look quite similar in Storybook — they seem to show the same fallback message or UI. I'm assuming this is expected because the Overview component handles these cases in a similar way with mock data, but please let me know if I might have missed something!

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 31, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: 1012Charan / name: Charan Vengala (bd534b2)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label May 31, 2025
@k8s-ci-robot
Copy link
Contributor

Welcome @1012Charan!

It looks like this is your first PR to kubernetes-sigs/headlamp 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/headlamp has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 31, 2025
@1012Charan 1012Charan force-pushed the add-overview-story-variants branch from 0b42c67 to 752234f Compare June 12, 2025 12:16
Copy link
Contributor

@skoeva skoeva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks solid, would be nice to have screenshots in the description for the new stories

Also good to rebase against main to pull the latest changes

@1012Charan
Copy link
Contributor Author

Thanks for the suggestion @skoeva! I've updated this branch added the screenshots, but I noticed that all three states — Empty, Loading, and Error — look almost the same in Storybook. I’m not sure if that’s okay or if I might have missed something, so please let me know if any changes are needed!

skoeva
skoeva previously requested changes Jul 1, 2025
Copy link
Contributor

@skoeva skoeva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The similar states should be fine, I would just squash the commits once you're done rebasing. Make sure to update the snapshots too: run make frontend-test locally and you can see which are failing

@1012Charan 1012Charan force-pushed the add-overview-story-variants branch from e16f1c7 to 9bf2886 Compare July 3, 2025 12:37
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 3, 2025
@1012Charan
Copy link
Contributor Author

@skoeva I've rebased the branch, squashed the commits, and updated the snapshots as requested.

Since this was my first time working with things like squash and rebase, it was an exciting learning experience — so if I missed anything or made mistakes, please feel free to point them out.

Also, while running the tests, I noticed a failure in the LoadingState due to it simulating an endless request. If you'd like, I can update it to use a timeout-based delay instead.

@1012Charan 1012Charan force-pushed the add-overview-story-variants branch from b6143d3 to 25e32f4 Compare July 3, 2025 12:51
@skoeva
Copy link
Contributor

skoeva commented Jul 3, 2025

sure, no need to mock an endless request. the frontend test still seems to be failing, are you seeing something different locally?

@1012Charan
Copy link
Contributor Author

1012Charan commented Jul 4, 2025

@skoeva You're right — I rechecked and the LoadingState test is still failing on my side too.

I think it's because the mocked API requests never resolve (to simulate loading), so the test waits indefinitely and throws a waiting error. Would it be okay to skip this story in the tests using:

LoadingState.parameters = {
...LoadingState.parameters,
chromatic: { disableSnapshot: true },
};

Also, another issue was simple-eval causing the frontend tests to fail is now fixed — I had missed installing it initially, but after adding it, the other tests are passing fine.

Let me know if this approach for the loading story makes sense or if you'd prefer a different fix!

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 24, 2025
@joaquimrocha
Copy link
Contributor

@skoeva Can you check this one?

Copy link
Contributor

@skoeva skoeva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will test the frontend locally and I can try to help push up a fix for the CI

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2025
@skoeva skoeva force-pushed the add-overview-story-variants branch 3 times, most recently from c80f58e to b7078e6 Compare August 13, 2025 02:49
@skoeva skoeva changed the title Expand Overview stories with empty, loading, and error states frontend: cluster: Add Overview stories Aug 13, 2025
@skoeva
Copy link
Contributor

skoeva commented Aug 13, 2025

@1012Charan Fixed the CI failures and removed the unnecessary changes, feel free to test locally and lmk if you notice anything

@skoeva skoeva self-assigned this Aug 13, 2025
@1012Charan
Copy link
Contributor Author

1012Charan commented Aug 14, 2025

Hi @skoeva ,
I pulled the latest changes and ran the frontend tests locally, and I'm happy to say they're all passing now! So, it looks like those CI issues you mentioned are all cleared up from my end.
I also took a look at the new Overview stories in Storybook (EmptyState, LoadingState, and ErrorState). As we talked about, they present a pretty similar visual output, but I totally get that's the intended behavior for these states.
Everything seems to be in order on my side. Just let me know if there's anything else you'd like me to verify or if you need any more help with this PR.
Thanks!

@skoeva skoeva force-pushed the add-overview-story-variants branch from b7078e6 to 68cddcb Compare August 14, 2025 14:23
Co-authored-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
Co-authored-by: 1012Charan <charanvengala@gmail.com>
@skoeva skoeva force-pushed the add-overview-story-variants branch from 68cddcb to bd534b2 Compare August 14, 2025 14:26
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 thanks folks

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 1012Charan, illume

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 14, 2025
@illume illume dismissed stale reviews from skoeva and joaquimrocha August 14, 2025 15:26

changes done

@illume illume merged commit 3473860 into kubernetes-sigs:main Aug 14, 2025
11 of 12 checks passed
@illume illume added this to the v0.35.0 milestone Sep 2, 2025
@illume illume added frontend Issues related to the frontend quality Related to improving the quality of the app labels Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. frontend Issues related to the frontend quality Related to improving the quality of the app size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

Development

Successfully merging this pull request may close these issues.

6 participants