Skip to content

[Project Solar / Phase 1 / Migration] Table & AdvancedTable carbonization - #3988

Open
shleewhite wants to merge 34 commits into
project-solar/phase-1-main-feature-branchfrom
project-solar/phase-1/shleewhite/table-carbonization
Open

[Project Solar / Phase 1 / Migration] Table & AdvancedTable carbonization#3988
shleewhite wants to merge 34 commits into
project-solar/phase-1-main-feature-branchfrom
project-solar/phase-1/shleewhite/table-carbonization

Conversation

@shleewhite

@shleewhite shleewhite commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

📌 Summary

If merged, this PR would carbonize the Table component and adds the relevant component tokens. It also adds a new state for the row that toggles the isSelected class.

Note: for reorderable columns, the HTML element to show the preview column as you're dragging is appended to the DOM so it doesn't show as themed on the carbonization page. To validate that it works correctly, check the normal AdvancedTable showcase page and change the theme.

:copilot: Copilot instructions

Read through similar carbonization pages under showcase/app/components/page-carbonization and compare those to the changes made here to ensure this PR aligns with previous carbonization efforts

🔗 External links

Jira ticket: HDS-6104


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

📋 PCI review checklist
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • If applicable, I've worked with GRC to document the impact of any changes to security controls.
    Examples of changes to controls include access controls, encryption, logging, etc.
  • If applicable, I've worked with GRC to ensure compliance due to a significant change to the in-scope PCI environment.
    Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.

@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hds-showcase Ready Ready Preview Aug 3, 2026 4:57pm
hds-website Ready Ready Preview Aug 3, 2026 4:57pm

Request Review

@didoo
didoo force-pushed the project-solar/phase-1/shleewhite/table-carbonization branch from 4396ca7 to 0dce377 Compare July 9, 2026 19:14
@shleewhite
shleewhite requested a review from Copilot July 13, 2026 20:30
@shleewhite shleewhite changed the title [WIP] [Project Solar / Phase 1 / Migration] Table carbonization [Project Solar / Phase 1 / Migration] Table carbonization Jul 13, 2026
@shleewhite
shleewhite marked this pull request as ready for review July 13, 2026 20:47
@shleewhite
shleewhite requested a review from a team as a code owner July 13, 2026 20:47

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@didoo didoo 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.

Pre-approving (only a minor thing to fix, and a nitpick)

Note: I have reviewed mainly the code, since @dchyun has already done a fine-combed visual review

Comment thread packages/tokens/src/products/shared/advanced-table.json Outdated
"color": {
"focus": {
"$type": "color",
"$value": "{core.color.blue-300}"

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.

@jorytindall @heatherlarsen do you remember why these colors were used?

},
"hover": {
"$type": "color",
"$value": "#b1b1b5"

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.

@jorytindall @heatherlarsen do you remember why these colors were used?

"surface": {
"color": {
"$type": "color",
"$value": "rgba(204, 227, 254, 30%)"

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.

@jorytindall @heatherlarsen do you remember why these colors were used?

Comment thread packages/components/src/styles/components/advanced-table.scss

@dchyun dchyun 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.

🎉🎉🎉

@jorytindall jorytindall 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.

Sorry for this barrage of comments that I wasn't really sure what to pin to. I'll try to organize them categorically.

Scroll indicator

We're losing the scroll indicator gradient in the AdvancedTable on the dark Carbon themes. There isn't really a Figma token for this, so LMK how it's best to define that.

Reordering/resizing

There may have been a few bugs introduced in the AdvancedTable:

  • the reordering button displays on the hover action for the column resizing feature.
Image - When I select "Move column" from the context menu, the drag handle displays, but is not focused (in the production branch it does do this)

Empty state

There's an issue with the empty state (either in the showcase examples, or otherwise), where hover the tooltip there's a z-index issue causing the empty state to overlap the tooltip. Additionally, the header is scrollable. However, the scroll only appears after the tooltip has been interacted with. I wasn't able to reproduce it in the main branch, but there also aren't tooltips in any of the examples.
Image


.hds-table__th {
padding: $hds-table-cell-padding-medium;
padding: var(--hds-table-header-cell-padding-vertical) var(--hds-table-cell-padding-horizontal)

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.

[question] I'm not quite sure where to add this, but the rendered height short HDS table is a bit taller (~2px) than the Carbon table and I think it's because of a 2px difference in the line height of the text. Is this something worth indexing on, or would we consider this a bit nit-picky?


// striped rows

.hds-table--striped & {

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.

[issue] When I apply sorting to a table that is striped, the alternating striping seems to be ignored and it's possible to have two adjacent rows that are the same color.

Image

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.

Disregard this, it's because the selected row has a different surface color that I didn't realize 🤦

--hds-table-cell-surface-color-striped-hover: rgba(141, 141, 141, 0.12);
--hds-table-header-button-border-color-active: rgba(0, 0, 0, 0);
--hds-table-header-button-border-color-hover: rgba(0, 0, 0, 0);
--hds-table-header-button-foreground-color-sorted-active: #161616;

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.

[issue] I'm seeing a slight inconsistency in the tooltip button and the sort button foreground colors. In the g0 theme:

  • The resting/default foreground color for the tooltip button and the indeterminate sort is #525252
  • The resting/default foreground color for the descending/ascending sort button is #161616

I think all of these should have the same resting/default foreground color, and this is what is reflected in Figma. Even though these interactive elements are different from how Carbon implements these features.

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.

Actually looks like this might be an issue with the secondary-muted variant (sorry if this comment is in a weird spot, kind of hard to know where it's best to comment).

@dchyun

dchyun commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@jorytindall

It looks like the reordering / resizing and empty state bug you caught are both ones present on main today. We could address those in a separate bug fix PR and merge it into main directly.

CC: @shleewhite

@dchyun

dchyun commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@jorytindall

It looks like the reordering / resizing and empty state bug you caught are both ones present on main today. We could address those in a separate bug fix PR and merge it into main directly.

CC: @shleewhite

Created a JIRA ticket for this https://hashicorp.atlassian.net/browse/HDS-6573

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants