🐛 Address Copilot comments: DEV badge a11y, navbar style, ESC handler churn#4171
🐛 Address Copilot comments: DEV badge a11y, navbar style, ESC handler churn#4171clubanderson merged 1 commit intomainfrom
Conversation
- Navbar: Remove redundant inline style={{ top: 0 }}, use Tailwind
top-0 class instead (#4167)
- DEV badge: Add aria-label + role="status", match lg: breakpoint
with title visibility (#4167)
- FeedbackModal: Use refs for dirty check in handleClose to prevent
keydown listener churn on every keystroke (#4161)
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
kubestellar-prow
bot
added
the
dco-signoff: yes
|
[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 |
kubestellar-prow
bot
added
the
size/S
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Updates the navbar and feedback modal to address prior review comments around styling, accessibility, and event-handler churn.
Changes:
- Replaces redundant inline navbar positioning with Tailwind classes.
- Improves DEV badge accessibility metadata.
- Stabilizes
FeedbackModalclose handler by moving “dirty check” inputs into refs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| web/src/components/layout/navbar/Navbar.tsx | Removes inline top: 0 style, adjusts DEV badge visibility breakpoint, and adds a11y attributes. |
| web/src/components/feedback/FeedbackModal.tsx | Uses refs for dirty-check values so handleClose stays stable (less ESC handler churn). |
| className="hidden lg:inline-flex items-center justify-center px-1.5 py-0.5 text-[9px] font-bold uppercase tracking-wider bg-green-500/20 text-green-400 border border-green-500/30 rounded-full" | ||
| title="Development mode" | ||
| aria-label="Development mode" | ||
| role="status" |
There was a problem hiding this comment.
role=\"status\" creates a live region (implicit aria-live=\"polite\") intended for dynamic status updates. Since the DEV badge appears to be static, this can cause unnecessary announcements for screen reader users. Consider removing role=\"status\" and rely on aria-label/visible text, or only use a live region if the badge’s content/state can change at runtime.
| role="status" |
| {__DEV_MODE__ && ( | ||
| <span | ||
| className="hidden sm:inline-flex items-center justify-center px-1.5 py-0.5 text-[9px] font-bold uppercase tracking-wider bg-green-500/20 text-green-400 border border-green-500/30 rounded-full" | ||
| className="hidden lg:inline-flex items-center justify-center px-1.5 py-0.5 text-[9px] font-bold uppercase tracking-wider bg-green-500/20 text-green-400 border border-green-500/30 rounded-full" |
There was a problem hiding this comment.
This changes the DEV badge visibility from sm to lg (hidden sm:inline-flex → hidden lg:inline-flex), which is a functional UI behavior change beyond the stated 'DEV badge accessibility' scope. If this is intentional, it should be called out in the PR description; otherwise revert to the previous breakpoint and keep the a11y-only changes.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 1 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
Addresses comments from #4160, #4161, #4167: redundant inline style, DEV badge accessibility, FeedbackModal ESC handler ref optimization.