fix: correct badge hover colors and completed job status badge colors#290
fix: correct badge hover colors and completed job status badge colors#290pggdev wants to merge 1 commit into
Conversation
Signed-off-by: pggdev <princegupta.ns153@gmail.com>
![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 standardizes status handling across the dashboard components (jobs, podgroups, pods, and queues) by normalizing status phases to lowercase and adding hover effects to the status badges. Feedback on these changes highlights two issues: first, the fallback badge style in jobs/columns.tsx defaults to red, which incorrectly styles unknown statuses as failures instead of gray; second, the backend filtering in helpers.ts fails to match podgroups with undefined phases when filtering by 'unknown' because it does not default undefined phases to 'unknown' like the frontend does.
| status === "completed" | ||
| ? "bg-green-100 text-green-800" | ||
| ? "bg-green-100 text-green-800 hover:bg-green-100/60" | ||
| : status === "running" | ||
| ? "bg-blue-100 text-blue-800" | ||
| ? "bg-blue-100 text-blue-800 hover:bg-blue-100/60" | ||
| : status === "pending" | ||
| ? "bg-yellow-100 text-yellow-800" | ||
| : "bg-red-100 text-red-800" | ||
| ? "bg-yellow-100 text-yellow-800 hover:bg-yellow-100/60" | ||
| : "bg-red-100 text-red-800 hover:bg-red-100/60" |
There was a problem hiding this comment.
The fallback badge style defaults to red (bg-red-100 text-red-800 hover:bg-red-100/60), which means any unhandled or unknown status will be styled as a failure. This is inconsistent with jobs-management.tsx where unknown is styled with a gray background.
We should explicitly handle failed and terminated statuses as red, and default to gray for unknown or other unexpected statuses.
status === "completed"
? "bg-green-100 text-green-800 hover:bg-green-100/60"
: status === "running"
? "bg-blue-100 text-blue-800 hover:bg-blue-100/60"
: status === "pending"
? "bg-yellow-100 text-yellow-800 hover:bg-yellow-100/60"
: status === "failed" || status === "terminated"
? "bg-red-100 text-red-800 hover:bg-red-100/60"
: "bg-gray-100 text-gray-800 hover:bg-gray-100/60"
| if (status && status !== "all" && pg.status?.phase?.toLowerCase() !== status) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
When filtering by status, if the user selects unknown, the backend filter will fail to match podgroups with an undefined phase. This is because pg.status?.phase?.toLowerCase() evaluates to undefined, and undefined !== "unknown" is true, causing them to be filtered out.
To align with the frontend mapping (which defaults undefined phase to 'unknown'), we should normalize the phase to 'unknown' on the backend before comparing.
| if (status && status !== "all" && pg.status?.phase?.toLowerCase() !== status) { | |
| return false; | |
| } | |
| const pgStatus = pg.status?.phase?.toLowerCase() || "unknown"; | |
| if (status && status !== "all" && pgStatus !== status) { | |
| return false; | |
| } |
de6p
left a comment
de6p
left a comment
There was a problem hiding this comment.
Thanks for the PR.
lgtm 🚀
cc @JesseStutler
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: de6p 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 |
3 participants
Summary
fixes #289
Replace the default red hover color used by the affected badges with the intended theme hover color.
Normalize job status values before applying the badge color mapping.
Ensure the Completed job status resolves to the configured green badge color instead of falling back to the default red style.
Screenshots:
Before
After