-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
frontend: add Chooser componnent stories #2824
base: main
Are you sure you want to change the base?
Conversation
frontend/src/components/cluster/__snapshots__/Chooser.LoadingClusters.stories.storyshot
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice changes, thanks! could you take a look into LoadingClusters story and why there are clusters in the output?
78d6cc3
to
9bc15bd
Compare
9bc15bd
to
326d0d1
Compare
frontend/src/lib/k8s/index.ts
Outdated
const allClusters = _.cloneDeep(state.allClusters || {}); | ||
|
||
if (clusters === null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this changes the behavior of this function and breaks logic for statelessClusters branch. could you revert the change to this file please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, i was expecting something might break due to this
@sniok I did this because this function useClustersConf
would always returns object || {}
and never null
, therefore
loading state
would never be achieved IMO, refering these 2 lines:
https://github.com/headlamp-k8s/headlamp/blob/main/frontend/src/components/cluster/Chooser.tsx#L417
Therefore, Removing loading state story for now
326d0d1
to
f2a8b4a
Compare
Signed-off-by: Faakhir30 <[email protected]>
f2a8b4a
to
1756226
Compare
Force pushed again, since e2e tests timed out after 13 min |
References #931
Added clusterChooser components stories.
Heres snapshots video of current state:
Screencast from 2025-02-01 04-10-54.webm