Skip to content

refactor(ui): ColumnGridTableRow render-props → compound cell slots (C3) - #31159

Open
harsh-vador wants to merge 1 commit into
mainfrom
refactor/c3-columngrid-row-compound
Open

refactor(ui): ColumnGridTableRow render-props → compound cell slots (C3)#31159
harsh-vador wants to merge 1 commit into
mainfrom
refactor/c3-columngrid-row-compound

Conversation

@harsh-vador

Copy link
Copy Markdown
Contributor

Problem

ColumnGridTableRow took five required renderXCell function props (renderColumnNameCell, renderPathCell, renderDescriptionCell, renderTagsCell, renderGlossaryTermsCell) — the render-prop proliferation flagged by audit C3.

Fix

Compound ColumnGridTableRow.Cell slot API (via the select.tsx cast + static-attach idiom). The parent passes pre-rendered cell content as children keyed by columnId; the row collects them into an id→content map and places each in the matching Table.Cell.

<ColumnGridTableRow entity={row} tableColumns={tableColumns} >
  <ColumnGridTableRow.Cell columnId="columnName">{renderColumnNameCellFinal(row)}</ColumnGridTableRow.Cell>
  <ColumnGridTableRow.Cell columnId="description">{renderDescriptionCellAdapter(row)}</ColumnGridTableRow.Cell></ColumnGridTableRow>
  • The row keeps all layout / indent / pending-loader / style logic.
  • dataType stays internal (static row data), as before.
  • Behaviour unchanged — the single caller already builds each cell with memoised useCallback adapters; they're now invoked at the call site, so each still renders once (no double-call).
  • Adding a column is now a new child, not a new required prop.

Testing

  • Jest: ColumnGridTableRow.test.tsx rewritten to the slot API — 3 tests pass (slot placement, pending loader, parent/child row typing).
  • tsc --noEmit clean (the single caller type-checks against the new API); lint:base 0 errors; prettier/organize-imports applied.

Part of the UI Quality Audit — open-metadata/openmetadata-collate#5442, C3 under open-metadata/openmetadata-collate#5449.

🤖 Generated with Claude Code

ColumnGridTableRow required five renderXCell function props
(renderColumnNameCell/PathCell/DescriptionCell/TagsCell/GlossaryTermsCell).
Replace them with a compound `ColumnGridTableRow.Cell` slot API: the parent
passes pre-rendered cell content as children keyed by columnId, collected
into an id→content map. Adding a column is now a new child, not a new
required prop.

The row keeps all layout/indent/pending-loader/style logic; `dataType`
stays internal (static row data). Behaviour is unchanged — the single
caller (ColumnGrid.component.tsx) already builds each cell with memoised
adapters, now invoked at the call site so each still renders once.

Refs #5449

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@harsh-vador
harsh-vador requested a review from a team as a code owner August 7, 2026 07:06
@github-actions

github-actions Bot commented Aug 7, 2026

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 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

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!

@gitar-bot

gitar-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Refactors ColumnGridTableRow from render props to compound cell slots to eliminate prop proliferation. No issues found.

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 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit bc26a56f9db7fc338cfa39108a8f0ae8fb10191f in Playwright run 31156565992, attempt 1.

✅ 550 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) 55m 49s

⏱️ Max setup 2m 48s · max shard execution 16m 31s · max shard-job elapsed before upload 20m 7s · reporting 4s

🌐 202.61 requests/attempt · 2.83 app boots/UI scenario · 5.25% common-shard skew

Optimization targets still in progress:

  • Browser traffic was 202.61 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.83 per UI scenario (1617 boots / 571 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 120 0 0 0 0 0
✅ Shard chromium-02 141 0 0 0 0 0
✅ Shard chromium-03 138 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 29 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 7, 2026
@harsh-vador harsh-vador added the safe to test Add this label to run secure Github workflows on PRs label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 42 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 42 warning(s) across 3 changed file(s).

Count Rule
13 react-hooks/exhaustive-deps
7 sonarjs/cyclomatic-complexity
7 i18next/no-literal-string
6 sonarjs/no-duplicate-string
5 sonarjs/no-nested-functions
2 sonarjs/cognitive-complexity
1 jsx-a11y/no-autofocus
1 sonarjs/no-identical-functions
All findings
Location Rule Message
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:421:16 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:445:13 jsx-a11y/no-autofocus The autoFocus prop should not be used, as it can reduce usability and accessibility for users.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:559:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":559,"column":3,"endLine":559,"endColumn
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:754:28 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 47 to the 15 allowed.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:754:28 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 24 which is greater than 10 authorized.","cost":14,"secondaryLocations":[{"line":754,"column":27,"endLine":754,"endColu
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:953:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'createStructChildRows' and 'getUniquePaths'. Either include them or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:974:62 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 13 which is greater than 10 authorized.","cost":3,"secondaryLocations":[{"line":974,"column":61,"endLine":974,"endColum
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1067:33 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":1067,"column":32,"endLine":1067,"endCol
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1114:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 't'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1223:33 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 15 which is greater than 10 authorized.","cost":5,"secondaryLocations":[{"line":1223,"column":32,"endLine":1223,"endCol
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1262:23 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1263:42 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1357:42 sonarjs/no-identical-functions Update this function so that its implementation is not identical to the one on line 1292.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1412:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1484:5 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1502:49 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1502:49 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 14 which is greater than 10 authorized.","cost":4,"secondaryLocations":[{"line":1502,"column":48,"endLine":1502,"endCol
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1614:24 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1618:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1627:29 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":1627,"column":28,"endLine":1627,"endCol
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1741:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1783:16 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1805:36 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1812:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:1934:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:2034:61 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:2044:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:2166:5 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:2199:5 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'columnGridListing'. Either include it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:2280:47 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:2293:47 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/ColumnGrid.component.tsx:2303:6 react-hooks/exhaustive-deps React Hook useMemo has an unnecessary dependency: 'columnGridListing.allRows'. Either exclude it or remove the dependency array.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.test.tsx:48:65 i18next/no-literal-string disallow literal string: loader
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.test.tsx:68:11 i18next/no-literal-string disallow literal string: name
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.test.tsx:71:11 i18next/no-literal-string disallow literal string: path
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.test.tsx:74:11 i18next/no-literal-string disallow literal string: desc
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.test.tsx:77:11 i18next/no-literal-string disallow literal string: tags
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.test.tsx:80:11 i18next/no-literal-string disallow literal string: glossary
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.test.tsx:95:31 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.test.tsx:114:17 i18next/no-literal-string disallow literal string: name
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.tsx:91:11 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.tsx:120:6 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'isChildRow'. Either include it or remove the dependency array.

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

make ui-checkstyle-changed

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 66%
66.22% (78153/118019) 50.22% (47203/93984) 51.43% (14221/27647)

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

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