feat(repo-health): add clickable health score breakdown panel#306
feat(repo-health): add clickable health score breakdown panel#306mallya-m wants to merge 6 commits into
Conversation
|
@mallya-m is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Priyanshu-byte-coder |
|
The health panel feature looks great — clean component, proper CSS vars, good a11y. However PR #303 (sortable columns) just merged and it touched the same area of Please rebase onto When resolving conflicts in Once rebased and pushed, I'll merge. |
7826005 to
00c8d13
Compare
|
@Priyanshu-byte-coder conflict resolved and rebased onto main — ready for |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Good feature. Two accessibility issues:
1. Dialog missing aria-labelledby
role="dialog" aria-modal="true" without aria-labelledby — screen readers won't know the dialog's title. Add an id to the <h2> and reference it:
<div role="dialog" aria-modal="true" aria-labelledby="health-panel-title">
...
<h2 id="health-panel-title" ...>Health Breakdown</h2>2. No Escape key to close
Modal panels should close on Escape. Add to RepoHealthPanel:
useEffect(() => {
const handler = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose(); };
document.addEventListener('keydown', handler);
return () => document.removeEventListener('keydown', handler);
}, [onClose]);Everything else is solid — proper aria-hidden on overlay, aria-label on close button, CSS vars for layout colors, and the score-bar semantic colors (green/yellow/red) are appropriate for health indicators.
29c9992 to
e4b1992
Compare
|
Resolved the merge conflict and synced Also addressed the requested accessibility updates:
All checks are passing locally now. |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
- Hardcoded colors — bg-green-500, bg-yellow-500, bg-red-500 in ScoreBar and text-green-400, text-yellow-400, text-red-400 in grade badge. Use CSS vars (var(--success), var(--destructive), or var(--accent)). 2. supabase/.temp/cli-latest committed — local Supabase temp file must not be in the repo. Add supabase/.temp/ to .gitignore and remove from branch. 3. .gitignore shows as binary diff — investigate encoding corruption. 4. Missing EOF newlines on RepoHealthPanel.tsx and TopRepos.tsx. 5. No-health-data fallback removed — repos with no health score now show nothing. Restore the -- fallback badge. 6. package-lock.json churn — unrelated npm version artifacts. Revert.
|
@Priyanshu-byte-coder all 6 issues addressed:
Ready for merge! |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Issues found in this PR:
-
Missing EOF newline — add a trailing newline to all modified files.
-
Raw Tailwind color classes — replace
text-red-*/bg-red-*withtext-[var(--destructive)]/ appropriate CSS var equivalents. All colors must use CSS variables for theme support.
16a258b to
6bcb5a8
Compare
|
@Priyanshu-byte-coder addressed both issues:
Ready for merge! |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Two issues:
- Missing EOF newline
- Hardcoded fallback hex in CSS vars —
var(--warning, #ca8a04)— remove the fallback. The var should be defined in globals.css. Since--warningis now defined in main (via merged PRs), just usevar(--warning).
What does this PR do?
Makes the repo health score badge in TopRepos clickable. Clicking opens a panel showing a per-dimension breakdown of the score with actionable improvement tips.
Related issue
Closes #256
Changes made
RepoHealthPanel.tsxcomponent with 5 dimension breakdownTopRepos.tsxa clickable button that opens the panel/api/metrics/repo-health— no new API callsHow to test
Screenshots