Skip to content

fix(warehouses): make WarehousesList table rows keyboard-accessible + clean dead .clickable-row CSS [Phase 2g]#637

Merged
barach6662001-bit merged 1 commit intomainfrom
replit/phase-2g-warehouseslist-a11y
Apr 24, 2026
Merged

fix(warehouses): make WarehousesList table rows keyboard-accessible + clean dead .clickable-row CSS [Phase 2g]#637
barach6662001-bit merged 1 commit intomainfrom
replit/phase-2g-warehouseslist-a11y

Conversation

@barach6662001-bit
Copy link
Copy Markdown
Owner

Phase 2g — WarehousesList AntD table-row keyboard accessibility + dead-CSS cleanup

Closes Finding 2 (medium-risk) and Finding 5 (medium-risk, CSS) from the Phase 2e audit. This PR is different from #629/#633/#636 because the rows are AntD <tr> elements rendered through the onRow callback, not plain <div onClick>s.

Before

onRow={(record) => ({
  onClick: () => navigate(`/warehouses/items?warehouse=${record.id}`)
})}
rowClassName={() => 'clickable-row'}

Mouse-clickable (cursor: pointer + hover bg) but no role, no tabIndex, no onKeyDown, no accessible name. Plus theme/global.css had two .clickable-row rules — a properly-scoped one (.ant-table-tbody > tr.clickable-row) and a redundant global !important duplicate.

After (TSX)

onRow now returns the full a11y bundle; AntD spreads it onto the <tr> directly:

  • role="button" (matches FieldCard / OperationsTimeline / UpcomingPanel / FieldStatusCard convention)
  • tabIndex={0}
  • aria-label="{name}, {location || '—'}, {typeLabel}, {statusLabel}" — mirrors visible cells
  • onClick — existing navigate, preserved verbatim
  • onKeyDown — Enter / Space + preventDefault on Space; unrelated keys ignored

After (CSS)

New focus-visible block next to the existing scoped rules:

.ant-table-tbody > tr.clickable-row:focus { outline: none; }
.ant-table-tbody > tr.clickable-row:focus-visible {
  outline: 2px solid var(--brand);
  outline-offset: -2px;   /* see note */
}
.ant-table-tbody > tr.clickable-row:focus-visible > td {
  background: var(--brand-muted) !important;
}

Why outline-offset: -2px (deviation from the standard +2px): AntD's table container has overflow: hidden + border-radius. A positive offset gets clipped on the leftmost / outer columns. Insetting the ring keeps it visible on every row. The hover background mirror preserves hover/focus visual parity.

Dead-CSS cleanup: the global .clickable-row { cursor: pointer !important; } rule is removed. Pre-deletion verification: repo-wide rg "'clickable-row'|"clickable-row"" returned exactly one consumer (WarehousesList.tsx) which is fully covered by the scoped rule. A small inline comment in global.css documents the removal so future audits don't re-flag a phantom dead rule.

Why role="button" on a <tr>

This overrides the implicit role="row" in the table accessibility tree. Deliberate trade-off: for a row-as-navigation pattern, "this row is activatable" is more useful to AT users than strict table semantics, and it matches the rest of the codebase. The audit explicitly allows this.

Scope (per request)

In: WarehousesList.tsx, new WarehousesList.test.tsx, theme/global.css (focus-visible add + dead duplicate removal).
Out: NotificationBell, CommandPalette, AppLayout, Sidebar, legacy components/dashboard/*, Card / Surface API. The actions-cell <a onClick> is in the audit's false-positives / out-of-scope set and is not touched ("Do not sweep unrelated table rows"). No new dependencies, no route or API or business-logic changes, no redesign.

Tests (14, new file)

Group Cases
Render one row per warehouse; empty list → no warehouse-row buttons
Accessibility aria-label format with location; em-dash placeholder for empty location; typeGrain in label; inactive status in label; tabIndex=0; .clickable-row className preserved
Activation click; Enter; Space; Space preventDefault; ignored keys; per-row routing sanity

Validation

  • pnpm --filter frontend test130 / 130 (14 new + 116 existing)
  • npx tsc -b --noEmit → clean
  • npx eslint <changed files> → 0 errors (2 pre-existing warnings, unrelated to this PR)
  • npx vite build → green (✓ built in 34 s)

… clean dead .clickable-row CSS [Phase 2g]

  Closes Finding 2 (medium-risk) and Finding 5 (medium-risk, CSS
  duplicate) from the Phase 2e clickable-elements audit
  (docs/accessibility/clickable-elements-audit.md).

  Before
  ------
  pages/Warehouses/WarehousesList.tsx renders an AntD Table where
  each row navigates to its warehouse-balances page:

    onRow={(record) => ({
      onClick: () => navigate(`/warehouses/items?warehouse=${record.id}`)
    })}
    rowClassName={() => 'clickable-row'}

  The row was clickable by mouse (cursor:pointer + hover background)
  but had no role, no tabIndex, no key handler, no accessible name.
  Keyboard users could not reach or activate the row; screen readers
  only heard the cell-by-cell column readout with no destination
  context.

  Additionally, theme/global.css carried two .clickable-row rules:
    - line 481-486: scoped .ant-table-tbody > tr.clickable-row
      (cursor + hover background) — the proper one
    - line 506-507: global .clickable-row { cursor: pointer
      !important; } — duplicate cursor rule, with !important. The
      audit flagged this as redundant.

  After (TSX)
  -----------
  The onRow callback now returns the full a11y bundle, which AntD
  spreads onto the <tr> element directly:

    - role="button"      (consistent with the rest of the codebase —
                          FieldCard, OperationsTimeline,
                          UpcomingPanel, FieldStatusCard)
    - tabIndex={0}       (keyboard reachable)
    - aria-label         "{name}, {location || '—'}, {typeLabel},
                          {statusLabel}" — mirrors the visible cells
    - onClick            existing navigate (preserved verbatim)
    - onKeyDown          Enter and Space activate; Space calls
                          preventDefault to suppress page scroll;
                          unrelated keys (Tab, Escape, Shift, arrows)
                          are ignored

  After (CSS)
  -----------
  A new :focus-visible block lives next to the existing scoped
  .clickable-row rules in theme/global.css:

    .ant-table-tbody > tr.clickable-row:focus { outline: none; }
    .ant-table-tbody > tr.clickable-row:focus-visible {
      outline: 2px solid var(--brand);
      outline-offset: -2px;
    }
    .ant-table-tbody > tr.clickable-row:focus-visible > td {
      background: var(--brand-muted) !important;
    }

  Note the deliberate **negative** outline-offset: AntD's table
  container has overflow:hidden + border-radius, so a positive
  offset gets clipped on the leftmost / outer columns. Insetting
  the ring keeps it visible on every row. The hover background is
  mirrored on focus-visible cells so keyboard and mouse focus look
  identical (preserves hover/focus parity).

  The dead duplicate
    .clickable-row { cursor: pointer !important; }
  is removed. Pre-deletion grep across frontend/src for
  'clickable-row' confirmed exactly one consumer
  (WarehousesList.tsx) which is fully covered by the scoped rule
  above. No other rule depends on the !important variant.

  Notes / scope
  -------------
  - role="button" on a <tr> overrides its implicit role="row" inside
    the table accessibility tree. This is a deliberate trade-off:
    for a row-as-navigation pattern, "this row is activatable" is
    more useful to AT users than strict table semantics. The audit
    recommendation explicitly allows "role='button' or another
    appropriate role compatible with AntD table semantics".
  - The actions cell still contains <a onClick> with no href. That
    pattern is in the Phase 2e audit's false-positives /
    out-of-scope set; it is **not** touched here per the explicit
    scope ("Do not sweep unrelated table rows").
  - WarehousesList already used PremiumTable → DataTable → AntD
    Table v5; PremiumTable / DataTable spread {...props} so onRow is
    forwarded unchanged. No wrapper API change is needed.
  - No routes change, no API calls change, no business logic
    changes, no redesign, no new dependencies. NotificationBell,
    CommandPalette, AppLayout, Sidebar and the legacy
    components/dashboard/* files remain untouched.

  Tests
  -----
  New file: pages/Warehouses/__tests__/WarehousesList.test.tsx
  14 cases across 3 groups:

    render
      - one clickable row per warehouse
      - empty list renders no warehouse-row buttons (AntD pagination
        arrows are deliberately not asserted on)

    accessibility
      - aria-label = "{name}, {location}, {typeLabel}, {statusLabel}"
      - empty location renders as em-dash placeholder
      - typeGrain reflected in accessible name
      - inactive status reflected in accessible name
      - tabIndex=0 (keyboard reachable)
      - .clickable-row class preserved (focus-visible + cursor styles)

    activation
      - click navigates to /warehouses/items?warehouse={id}
      - Enter navigates
      - Space navigates
      - Space calls preventDefault
      - Tab / Escape / Shift / ArrowDown do nothing
      - each row routes to its own warehouse id (multi-row sanity)

  Validation
  ----------
    pnpm --filter frontend test  → 130 / 130 (14 new + 116 existing)
    npx tsc -b --noEmit          → clean
    npx eslint <changed files>   → 0 errors (2 pre-existing warnings,
                                    unrelated to this PR)
    npx vite build               → green (✓ built in 34 s)
@barach6662001-bit barach6662001-bit merged commit 800d38d into main Apr 24, 2026
3 checks passed
@barach6662001-bit barach6662001-bit deleted the replit/phase-2g-warehouseslist-a11y branch April 24, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant