Skip to content

chore(ui): consolidate app/appset tiles and tables#28873

Draft
blakepettersson wants to merge 1 commit into
argoproj:masterfrom
blakepettersson:refactor/definition-list-components
Draft

chore(ui): consolidate app/appset tiles and tables#28873
blakepettersson wants to merge 1 commit into
argoproj:masterfrom
blakepettersson:refactor/definition-list-components

Conversation

@blakepettersson

Copy link
Copy Markdown
Member

Use definition lists (<dl>) to make it more clear what the content is.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Title of the PR
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.
  • Optional. My organization is added to USERS.md.
  • Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

Copilot AI review requested due to automatic review settings July 23, 2026 11:41
@bunnyshell

bunnyshell Bot commented Jul 23, 2026

Copy link
Copy Markdown

❗ Preview Environment deployment failed on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Bundle Report

Changes will increase total bundle size by 1.3kB (0.01%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
argo-cd-ui-array-push 11.15MB 1.3kB (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: argo-cd-ui-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
main.*.js 1.3kB 3.61MB 0.04%

Copilot AI left a comment

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.

Pull request overview

This PR refactors the Applications/ApplicationSets list UI to consolidate the “tile” and “table row” metadata rendering into a shared definition-list (<dl><dt><dd>) primitive, aiming to make the “label: value” structure clearer while reusing existing styling utilities.

Changes:

  • Introduces shared EntryField / EntryFieldList components to render entry metadata as definition lists across tile + table variants.
  • Reworks Application/ApplicationSet tile and table-row markup to use the shared primitive and adds new SCSS to preserve legacy spacing/truncation behavior.
  • Adjusts table CSS to accommodate a flush-right actions menu layout and removes now-obsolete tile grid markup styles.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ui/src/app/applications/components/applications-list/entry-fields.tsx Adds shared <dl>-based primitives for rendering entry fields consistently across tile/table views.
ui/src/app/applications/components/applications-list/entry-fields.scss Adds layout/reset styles for the new definition-list rendering and table-specific grid placement rules.
ui/src/app/applications/components/applications-list/appset-tile.tsx Migrates ApplicationSet tile fields to the shared EntryFieldList rendering and improves record-level a11y grouping.
ui/src/app/applications/components/applications-list/appset-table-row.tsx Migrates ApplicationSet table row fields to EntryFieldList and updates layout structure.
ui/src/app/applications/components/applications-list/applications-tiles.scss Removes legacy tile grid wrapper styles now replaced by entry-fields.scss.
ui/src/app/applications/components/applications-list/applications-table.scss Updates action menu positioning rules and removes legacy per-row grid rules now handled by entry-fields.scss.
ui/src/app/applications/components/applications-list/application-tile.tsx Migrates Application tile fields to the shared EntryFieldList rendering and improves record-level a11y grouping.
ui/src/app/applications/components/applications-list/application-table-row.tsx Migrates Application table row fields to shared primitives and restructures the row (fav/actions/menu + flat <dl>).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ui/src/app/applications/components/applications-list/applications-table.scss Outdated
Copilot AI review requested due to automatic review settings July 23, 2026 22:39
@blakepettersson
blakepettersson force-pushed the refactor/definition-list-components branch from 0984e1b to 554bc24 Compare July 23, 2026 22:39

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment on lines +10 to +13
// To avoid re-deriving styling the list views already have, each <dt>/<dd> reuses the existing
// utility classes — the tile field classes, the table meta classes, and Foundation's own
// `show-for-xxlarge` visibility utility — so spacing, truncation and label visibility match the
// legacy markup. The only new CSS is the flex row + the <dl> 2-column grid.
Comment on lines +207 to +212
// When the <dl> IS the column (Source/Destination, Status), it doesn't receive Foundation's
// `.row > .columns` gutter, so apply that gutter to the cells instead — both sides of every
// cell, exactly as Foundation pads each column (gutter/2 ≈ 0.9375rem per side; adjacent cells'
// paddings combine into the gap between them). Matches column 1, whose gutter comes from its
// `small-4` wrapper. Column 1's <dl> (`.meta-rows`, not `.columns`) is untouched.
.entry-fields--table.columns {
Use definition lists (`<dl>`) to make it more clear what the content is.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Copilot AI review requested due to automatic review settings July 24, 2026 15:01
@blakepettersson
blakepettersson force-pushed the refactor/definition-list-components branch from 554bc24 to 198f009 Compare July 24, 2026 15:01

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

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.

2 participants