chore/fix: show refresh loading state in jobs and pod on refresh button#256
Conversation
Signed-off-by: gautamsidhwani29 <gautamsidhwani405@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Jobs and Pods components to utilize the loading state for the isRefreshing prop in the search bar, replacing hardcoded false values. The review feedback highlights that the loading state is currently overloaded in both components, as it tracks both the overall list fetching and individual item YAML fetching. This results in the refresh button displaying a loading state even when a user is only viewing details. It is recommended to split these into dedicated state variables to provide more accurate UI feedback.
| handleRefresh={fetchJobs} | ||
| fetchData={fetchJobs} | ||
| isRefreshing={false} // Update if needed | ||
| isRefreshing={loading} |
There was a problem hiding this comment.
The loading state is currently overloaded, as it tracks both the overall job list fetching (via fetchJobs) and the individual job YAML fetching (via handleJobClick). This causes the refresh button in the search bar to display "Refreshing..." and spin even when a user is only viewing job details. Consider using a dedicated state variable for list refreshing to provide more accurate UI feedback.
| handleRefresh={handleRefresh} | ||
| fetchData={fetchPods} | ||
| isRefreshing={false} // Update if needed | ||
| isRefreshing={loading} |
There was a problem hiding this comment.
The loading state is overloaded here as well, tracking both the pod list fetch and the YAML fetch in handlePodClick. This results in the refresh button showing a loading state during detail views. Splitting this into separate state variables for list loading and detail loading would improve the user experience and prevent misleading UI states.
2 participants
Summary
The refresh button had
isRefreshing={false}hardcoded, so the loading spinner and"Refreshing..."text never appeared during data fetches.Closes #255
Changes
loadingstate in:Jobs.jsxPods.jsx