Skip to content

feat(core): compact table size, radio single-selection, and fix the dead size prop - #31953

Open
harsh-vador wants to merge 1 commit into
mainfrom
feat/core-table-compact-radio
Open

feat(core): compact table size, radio single-selection, and fix the dead size prop#31953
harsh-vador wants to merge 1 commit into
mainfrom
feat/core-table-compact-radio

Conversation

@harsh-vador

@harsh-vador harsh-vador commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Groundwork for migrating the AntD tables onto TableV2.

size="compact"

The tables being migrated are dense — entity pages show 20+ rows at a time, and the existing sm/md heights push a third of them below the fold. Adds a third size (h-10 rows, h-8 header, px-4 py-2 cells) for them to map onto.

Radio for single selection

selectionMode="single" rendered a checkbox, which reads as "pick several". It now renders RadioButtonBase through React Aria's selection slot, so the behaviour stays React Aria's and only the visual changes.

The size prop never worked

value={{ size: context?.size ?? size, stickyHeader }}

TableContext defaulted to a filled-in { size: 'md' }, so context?.size always won and the prop was discarded — every table has been rendering at md regardless of what it asked for. The context now defaults to null, so an explicit prop wins and an enclosing TableCard is still inherited.

This surfaced from writing the unit test, not from review — worth knowing if any table looked denser in a design than it did in the app.

Verification

  • table.test.tsx added: all three sizes, the size default, single vs multiple selection
  • yarn vitest run — 25/25
  • tsc --noEmit clean, eslint clean

Note for reviewers

openmetadata-ui typechecks against this package's prebuilt dist/, not its source. Anything consuming compact needs a core build first, or it will fail to typecheck for a reason that has nothing to do with it.

🤖 Generated with Claude Code

Greptile Summary

The PR adds a compact table density, fixes explicit and inherited size resolution, and gives single-selection controls a radio-style appearance.

  • Adds compact header, row, cell, and selection-control sizing.
  • Resolves table size from an explicit prop, enclosing TableCard, or the default.
  • Adds tests covering sizes and single/multiple selection.
  • The single-selection visual currently remains exposed as a checkbox, and the new test file lacks its required license header.

Confidence Score: 4/5

The accessibility-semantic mismatch in single-selection tables should be fixed before merging; the missing test-file license header is also actionable.

Single-selection controls are now visually rendered as radio buttons but remain semantic checkboxes, causing assistive technology and keyboard interaction to disagree with the presented control.

Files Needing Attention: openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.tsx; openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.test.tsx

Important Files Changed

Filename Overview
openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.tsx Adds compact sizing and corrected context precedence, but the radio-style single-selection control retains checkbox accessibility semantics.
openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.test.tsx Adds useful size and selection coverage, but codifies the checkbox role for the radio visual and omits the required license header.

Reviews (1): Last reviewed commit: "feat(core): add compact table size, radi..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

Context used:

  • Context used - openmetadata-ui-core-components/CLAUDE.md (source)

…size prop

The AntD tables being migrated onto TableV2 are dense — entity pages show 20+
rows at a time, and the existing `sm`/`md` heights push a third of them below
the fold. Adds a `compact` size (h-10 rows, h-8 header, px-4/py-2 cells) for
them to map onto.

`selectionMode="single"` rendered a checkbox, which reads as "pick several".
It now renders RadioButtonBase through React Aria's selection slot, so the
behaviour stays React Aria's and only the visual changes.

Also fixes the `size` prop, which never had any effect: TableContext defaulted
to a filled-in `{ size: 'md' }`, so `context?.size ?? size` always resolved to
the context and discarded the prop. Every table has been rendering at `md`
regardless of what it asked for. The context now defaults to null, so an
explicit prop wins and an enclosing TableCard is still inherited.

Adds table.test.tsx covering all three sizes, the size default, and single vs
multiple selection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions github-actions Bot added the UI UI specific issues label Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

/**
* `compact` matches the density of the AntD tables this component replaces —
* entity pages show 20+ rows at a time and the `sm`/`md` heights push a third of
* them below the fold. See docs/antd-migration/table.md.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Comment references non-existent docs/antd-migration/table.md

The new doc comment points readers to docs/antd-migration/table.md, but that file does not exist (the directory only contains README.md, typography.md and button.md). The dangling reference will confuse anyone following it. Either add the table.md doc or drop/redirect the reference.

Was this helpful? React with 👍 / 👎

Comment on lines +387 to +401
{selectionMode === 'single' ? (
// A single-selection table reads as a radio group: picking a row
// clears the previous pick. React Aria still wires the control
// through the `selection` slot, so the visual is swapped rather
// than the behaviour.
<AriaCheckbox
className="tw:flex tw:items-center tw:outline-hidden"
slot="selection">
{({ isSelected, isDisabled, isFocusVisible }) => (
<RadioButtonBase
isDisabled={isDisabled}
isFocusVisible={isFocusVisible}
isSelected={isSelected}
size={size === 'md' ? 'md' : 'sm'}
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Edge Case: Single-selection row renders radio visual but checkbox a11y role

For selectionMode="single" the control is still an AriaCheckbox (slot="selection"), so it exposes role="checkbox" to assistive tech (the test even asserts getByRole('checkbox')). Screen-reader users hear "checkbox" while sighted users see a radio, a semantic mismatch versus the intended "pick one" affordance. If this is an accepted React Aria limitation, note it in the code comment; otherwise consider a radiogroup-based selection so the announced role matches the visual.

Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 2 findings

Adds compact table sizing, radio single-selection mode, and fixes the dead size context propagation. Update the documentation reference path and align the single-selection accessibility role with its radio visuals.

💡 Quality: Comment references non-existent docs/antd-migration/table.md

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.tsx:72

The new doc comment points readers to docs/antd-migration/table.md, but that file does not exist (the directory only contains README.md, typography.md and button.md). The dangling reference will confuse anyone following it. Either add the table.md doc or drop/redirect the reference.

💡 Edge Case: Single-selection row renders radio visual but checkbox a11y role

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.tsx:387-401

For selectionMode="single" the control is still an AriaCheckbox (slot="selection"), so it exposes role="checkbox" to assistive tech (the test even asserts getByRole('checkbox')). Screen-reader users hear "checkbox" while sighted users see a radio, a semantic mismatch versus the intended "pick one" affordance. If this is an accepted React Aria limitation, note it in the code comment; otherwise consider a radiogroup-based selection so the announced role matches the visual.

🤖 Prompt for agents
Code Review: Adds compact table sizing, radio single-selection mode, and fixes the dead size context propagation. Update the documentation reference path and align the single-selection accessibility role with its radio visuals.

1. 💡 Quality: Comment references non-existent docs/antd-migration/table.md
   Files: openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.tsx:72

   The new doc comment points readers to `docs/antd-migration/table.md`, but that file does not exist (the directory only contains README.md, typography.md and button.md). The dangling reference will confuse anyone following it. Either add the table.md doc or drop/redirect the reference.

2. 💡 Edge Case: Single-selection row renders radio visual but checkbox a11y role
   Files: openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.tsx:387-401

   For `selectionMode="single"` the control is still an `AriaCheckbox` (slot="selection"), so it exposes role="checkbox" to assistive tech (the test even asserts getByRole('checkbox')). Screen-reader users hear "checkbox" while sighted users see a radio, a semantic mismatch versus the intended "pick one" affordance. If this is an accepted React Aria limitation, note it in the code comment; otherwise consider a radiogroup-based selection so the announced role matches the visual.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 87e0b53e21af75699720ae44055e33bda926d798 in Playwright run 32721448947, attempt 1.

✅ 65 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 39m 3s

⏱️ Max setup 2m 46s · max shard execution 6m 39s · max shard-job elapsed before upload 9m 45s · reporting 3s

🌐 156.97 requests/attempt · 1.73 app boots/UI scenario · 0.00% common-shard skew

Optimization targets still in progress:

  • Application boot ratio was 1.73 per UI scenario (116 boots / 67 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 65 0 0 0 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@harsh-vador harsh-vador self-assigned this Aug 24, 2026
Comment on lines +392 to +403
<AriaCheckbox
className="tw:flex tw:items-center tw:outline-hidden"
slot="selection">
{({ isSelected, isDisabled, isFocusVisible }) => (
<RadioButtonBase
isDisabled={isDisabled}
isFocusVisible={isFocusVisible}
isSelected={isSelected}
size={size === 'md' ? 'md' : 'sm'}
/>
)}
</AriaCheckbox>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Radio visual retains checkbox semantics

When a table uses selectionMode="single", this branch renders a radio-style visual inside AriaCheckbox, so assistive technology announces a checkbox and keyboard users do not receive the interaction expected from the presented radio control.

@@ -0,0 +1,87 @@
import { render, screen, within } from '@testing-library/react';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 New test lacks license header

This new TSX file omits the repository-required Apache-2.0 license header, leaving it inconsistent with the source-licensing convention and subject to license-header validation failure.

Context Used: openmetadata-ui-core-components/CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@harsh-vador harsh-vador added the safe to test Add this label to run secure Github workflows on PRs label Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

❌ UI Checkstyle Failed

❌ Core Components - ESLint + Prettier

One or more core-component files have linting or formatting issues.

Affected files
  • openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.test.tsx
    • openmetadata-ui-core-components/src/main/resources/ui/src/components/application/table/table.tsx

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant