fix(a11y): EntityRow and MetricCard onClick — add role=button, tabIndex, onKeyDown (QUA-169)

[hungdqdesign]

Summary

Fixes keyboard accessibility for two widely-used reusable components:

EntityRow (click-only mode, no to prop):

• Add role="button" + tabIndex={0} + onKeyDown (Enter/Space)

MetricCard (onClick variant):

• Add role="button" + tabIndex={0} + onKeyDown (Enter/Space)

Related Issue

QUA-169: [A11Y] EntityRow and MetricCard onClick variants not keyboard-accessible

Acceptance Criteria

• EntityRow with onClick is keyboard-navigable
• MetricCard with onClick is keyboard-navigable
• Enter/Space trigger click action
• No regression for mouse/touch users
• WCAG 2.1 AA compliant

Impact

These are reusable components used across many pages — fix propagates broadly.

Test plan

• Tab to EntityRow → focus visible
• Press Enter/Space on EntityRow → action triggered
• Tab to MetricCard → focus visible
• Press Enter/Space on MetricCard → action triggered
• Mouse click still works normally on both

🤖 PM-coordinated PR (QUA-169)

[greptile-apps]

Greptile Summary

This PR adds keyboard accessibility (role="button", tabIndex={0}, onKeyDown Enter/Space) to the onClick-only variants of EntityRow and MetricCard — two widely-used reusable components. The direction is correct and the ARIA pattern is broadly sound, but there are two substantive issues with the onKeyDown implementation and a consistency gap with the design system's focus-ring style.

Key findings:

• Event-bubbling in onKeyDown (both components): Neither handler guards against bubbled events (e.target === e.currentTarget). If a focusable child element (e.g. a <button> or <a> in the trailing prop of EntityRow) is focused and the user presses Space, e.preventDefault() fires before the child's keyup-triggered click, hijacking the action and calling the outer onClick instead. See inline comments on EntityRow.tsx:70 and MetricCard.tsx:51.
• Missing design-system focus ring (both components): The project's interactive elements (see button.tsx) use outline-none + focus-visible:ring-[3px] to render a consistent focus ring. These new role="button" divs fall back to the browser default outline, which is visually inconsistent and may differ across environments. See inline comments on EntityRow.tsx:65 and MetricCard.tsx:46.
• PR description is missing a "thinking path": Per CONTRIBUTING.md, PR descriptions should include a top-down "thinking path" explaining what was changed, why it matters, benefits, and risks. Please add this section.
• No before/after screenshots: CONTRIBUTING.md asks for screenshots when there is a visible change. Focus-ring behaviour is a visible UI change — please include before/after screenshots (e.g. showing the focused state of an EntityRow with and without the fix).

Confidence Score: 3/5

• Safe to merge for mouse/touch users, but the keyboard fix has an event-bubbling gap that could misbehave when focusable children are present inside the components.
• The two changed UI files are straightforward and introduce no regressions for existing mouse/touch behaviour. However, both onKeyDown handlers lack a target === currentTarget guard, meaning Space/Enter keypresses on focusable children will incorrectly bubble up and trigger the outer onClick while also preventing the child's own default action. The missing focus-visible ring is an inconsistency but not a blocker. The PR description also lacks the required thinking path and screenshots per CONTRIBUTING.md.
• ui/src/components/EntityRow.tsx and ui/src/components/MetricCard.tsx — specifically the onKeyDown handlers on the role="button" divs.

Important Files Changed

Filename	Overview
ui/src/components/EntityRow.tsx	Adds role="button", tabIndex={0}, and onKeyDown handler to the onClick-only (no to prop) render path. The accessibility intent is correct, but there are two concerns: (1) the onKeyDown handler fires on bubbled events from focusable children in trailing, which can steal Space/Enter from inner buttons; (2) no explicit focus-visible ring class is added, leaving the focus indicator relying on browser defaults rather than the project's design-system ring style.
ui/src/components/MetricCard.tsx	Adds role="button", tabIndex={0}, and onKeyDown (Enter/Space) to the onClick-only render path. Logic is sound. Minor concern: no explicit focus-visible ring class, inconsistent with the design system's button focus pattern.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: ui/src/components/EntityRow.tsx
Line: 70

Comment:
**`onKeyDown` fires on events bubbled from focusable children**

The handler doesn't verify that the event originated on the root `<div>` itself. If `trailing` contains a focusable child element — such as an `<a>`, a `<button>`, or another `role="button"` node — and the user presses Space while focused on that child:

1. The keydown event bubbles up to this div.
2. The handler calls `e.preventDefault()`, which **suppresses the child button's subsequent `keyup`-triggered click** (browsers fire button clicks on Space `keyup`, and `preventDefault` on `keydown` cancels it).
3. The outer `onClick` fires instead of the intended inner action.

Add a target guard to restrict the handler to self-focus only. Replace the handler expression with one that checks `e.target === e.currentTarget` before acting.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/components/MetricCard.tsx
Line: 51

Comment:
**`onKeyDown` fires on events bubbled from child elements**

Same event-bubbling issue as in `EntityRow`: if any child inside `inner` becomes focusable (e.g. an icon wrapped in a link), a Space or Enter keydown bubbles up here. `e.preventDefault()` is called before the child's `keyup` click fires, hijacking the interaction and triggering the outer `onClick` instead.

Add a `e.target === e.currentTarget` check before the key guard to scope the handler strictly to the card's own focus.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/components/EntityRow.tsx
Line: 65-74

Comment:
**Missing focus-visible ring — inconsistent with design system**

All other interactive elements in this project (see `ui/src/components/ui/button.tsx`) pair `outline-none` with an explicit `focus-visible:ring-[3px]` class so the focus indicator matches the design system. Without it, this `<div>` falls back to the browser's default outline, which is visually inconsistent and can appear differently (or be suppressed) across browsers.

Consider adding the same `focus-visible:outline-none focus-visible:ring-[3px]` classes used by `<Button>` — conditioned on `isClickable` — alongside the `classes` variable.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/components/MetricCard.tsx
Line: 46-55

Comment:
**Missing focus-visible ring — inconsistent with design system**

Like the `EntityRow` change, this wrapper div will show the raw browser outline on focus rather than the project's design-system ring style (see `ui/src/components/ui/button.tsx` for the pattern). For a PR claiming WCAG 2.1 AA compliance, having a non-standard, browser-default focus ring is worth addressing to ensure consistent, predictable visual feedback across the UI.

Add `focus-visible:outline-none focus-visible:ring-[3px]` (plus an appropriate `rounded-lg` so the ring follows the card shape) to the wrapper `className`.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "fix(a11y): add role=..."}

[greptile-apps]

onKeyDown fires on events bubbled from focusable children

The handler doesn't verify that the event originated on the root <div> itself. If trailing contains a focusable child element — such as an <a>, a <button>, or another role="button" node — and the user presses Space while focused on that child:

1. The keydown event bubbles up to this div.
2. The handler calls e.preventDefault(), which suppresses the child button's subsequent keyup-triggered click (browsers fire button clicks on Space keyup, and preventDefault on keydown cancels it).
3. The outer onClick fires instead of the intended inner action.

Add a target guard to restrict the handler to self-focus only. Replace the handler expression with one that checks e.target === e.currentTarget before acting.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/components/EntityRow.tsx
Line: 70

Comment:
**`onKeyDown` fires on events bubbled from focusable children**

The handler doesn't verify that the event originated on the root `<div>` itself. If `trailing` contains a focusable child element — such as an `<a>`, a `<button>`, or another `role="button"` node — and the user presses Space while focused on that child:

1. The keydown event bubbles up to this div.
2. The handler calls `e.preventDefault()`, which **suppresses the child button's subsequent `keyup`-triggered click** (browsers fire button clicks on Space `keyup`, and `preventDefault` on `keydown` cancels it).
3. The outer `onClick` fires instead of the intended inner action.

Add a target guard to restrict the handler to self-focus only. Replace the handler expression with one that checks `e.target === e.currentTarget` before acting.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

onKeyDown fires on events bubbled from child elements

Same event-bubbling issue as in EntityRow: if any child inside inner becomes focusable (e.g. an icon wrapped in a link), a Space or Enter keydown bubbles up here. e.preventDefault() is called before the child's keyup click fires, hijacking the interaction and triggering the outer onClick instead.

Add a e.target === e.currentTarget check before the key guard to scope the handler strictly to the card's own focus.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/components/MetricCard.tsx
Line: 51

Comment:
**`onKeyDown` fires on events bubbled from child elements**

Same event-bubbling issue as in `EntityRow`: if any child inside `inner` becomes focusable (e.g. an icon wrapped in a link), a Space or Enter keydown bubbles up here. `e.preventDefault()` is called before the child's `keyup` click fires, hijacking the interaction and triggering the outer `onClick` instead.

Add a `e.target === e.currentTarget` check before the key guard to scope the handler strictly to the card's own focus.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Missing focus-visible ring — inconsistent with design system

All other interactive elements in this project (see ui/src/components/ui/button.tsx) pair outline-none with an explicit focus-visible:ring-[3px] class so the focus indicator matches the design system. Without it, this <div> falls back to the browser's default outline, which is visually inconsistent and can appear differently (or be suppressed) across browsers.

Consider adding the same focus-visible:outline-none focus-visible:ring-[3px] classes used by <Button> — conditioned on isClickable — alongside the classes variable.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/components/EntityRow.tsx
Line: 65-74

Comment:
**Missing focus-visible ring — inconsistent with design system**

All other interactive elements in this project (see `ui/src/components/ui/button.tsx`) pair `outline-none` with an explicit `focus-visible:ring-[3px]` class so the focus indicator matches the design system. Without it, this `<div>` falls back to the browser's default outline, which is visually inconsistent and can appear differently (or be suppressed) across browsers.

Consider adding the same `focus-visible:outline-none focus-visible:ring-[3px]` classes used by `<Button>` — conditioned on `isClickable` — alongside the `classes` variable.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Missing focus-visible ring — inconsistent with design system

Like the EntityRow change, this wrapper div will show the raw browser outline on focus rather than the project's design-system ring style (see ui/src/components/ui/button.tsx for the pattern). For a PR claiming WCAG 2.1 AA compliance, having a non-standard, browser-default focus ring is worth addressing to ensure consistent, predictable visual feedback across the UI.

Add focus-visible:outline-none focus-visible:ring-[3px] (plus an appropriate rounded-lg so the ring follows the card shape) to the wrapper className.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/components/MetricCard.tsx
Line: 46-55

Comment:
**Missing focus-visible ring — inconsistent with design system**

Like the `EntityRow` change, this wrapper div will show the raw browser outline on focus rather than the project's design-system ring style (see `ui/src/components/ui/button.tsx` for the pattern). For a PR claiming WCAG 2.1 AA compliance, having a non-standard, browser-default focus ring is worth addressing to ensure consistent, predictable visual feedback across the UI.

Add `focus-visible:outline-none focus-visible:ring-[3px]` (plus an appropriate `rounded-lg` so the ring follows the card shape) to the wrapper `className`.

How can I resolve this? If you propose a fix, please make it concise.