AdvancedTable - Column resizing #2849
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
zamoore
force-pushed
the
zamoore/hds-4585/AdvancedTable/column-resizing
branch
from
79a538c to
9bd84b3
Compare
zamoore
force-pushed
the
zamoore/hds-4585/AdvancedTable/column-resizing
branch
from
2d33b55 to
7e5bb77
Compare
| border-radius: 3px; | ||
|
|
||
| &:hover, | ||
| &.mock-hover { |
There was a problem hiding this comment.
| &.mock-hover { | |
| &.mock-hover, | |
| &:focus, | |
| &.mock-focus { |
[Question] Should the focus state also have these hover state styles? Right now the focus state does not have the white background or black icon.
There was a problem hiding this comment.
Good question, I'm not overly opinionated, but I do think that leaving out the white background and the change in icon color on focus is fine.
|
|
||
| &:focus, | ||
| &.mock-focus { | ||
| border-color: transparent; |
There was a problem hiding this comment.
[Question] When the menu is open, the hover state for the button does not have a border. Should it match the hover state for when it is closed? This is noticeable when you click to open the menu and the border gets removed from the button.
There was a problem hiding this comment.
I dropped a follow-up comment about this on this thread. When the menu is open, the focus ring should persist (at least that's how our other dropdown components function.
There was a problem hiding this comment.
[Suggestion] Can you add a section at the bottom for the ThContextMenu and show the different states as is done for the other buttons and icons?
| display: block; | ||
| width: 100%; | ||
| min-width: 30px; | ||
| overflow: hidden; |
There was a problem hiding this comment.
[Suggestion] Not sure if this is doable, but could we apply these styles limiting the header text to one line to all
column headers if any of the columns are resizable? In one example the last column is wrapping to two lines when the column before it is increased even though it doesn't need to. The table will overflow anyway so the column doesn't need to go to two lines.
Screen.Recording.2025-06-05.at.1.36.17.PM.mov
| export default class HdsAdvancedTableThContextMenu extends Component<HdsAdvancedTableThContextMenuSignature> { | ||
| originalColumnWidth?: string = this.args.column.width; | ||
|
|
||
| get classNames() { |
There was a problem hiding this comment.
[Nit] since it's only one class could just add it in the template
| } | ||
|
|
||
| .hds-advanced-table__th-context-menu .hds-dropdown-toggle-icon { | ||
| width: 24px; |
There was a problem hiding this comment.
[Suggestion] Might be worth creating a variable for this width and using it for all the buttons (tooltip, sort, expand)
| color: var(--token-color-foreground-faint); | ||
| background-color: transparent; | ||
| border: 1px solid transparent; | ||
| border-radius: 3px; |
There was a problem hiding this comment.
| border-radius: 3px; | |
| border-radius: var(--token-border-radius-x-small); |
| bottom: 0; | ||
| left: 0; | ||
| border: none; | ||
| border-radius: 3px; |
There was a problem hiding this comment.
| border-radius: 3px; | |
| border-radius: var(--token-border-radius-x-small); |
There was a problem hiding this comment.
Followed up on a few threads, I'm seeing another bug with the focus state (that @dchyun caught as well), and I'm still seeing a discrepancy in the border radius on the resizable column sorting table when a column is sorted.
| border-radius: 3px; | ||
|
|
||
| &:hover, | ||
| &.mock-hover { |
There was a problem hiding this comment.
Good question, I'm not overly opinionated, but I do think that leaving out the white background and the change in icon color on focus is fine.
|
|
||
| &:focus, | ||
| &.mock-focus { | ||
| border-color: transparent; |
There was a problem hiding this comment.
I dropped a follow-up comment about this on this thread. When the menu is open, the focus ring should persist (at least that's how our other dropdown components function.
📌 Summary
If merged, this PR will allow users to resize AdvancedTable columns.
🛠️ Detailed description
columnmodel alongside the existingtableandrowmodels.Added components
Hds::AdvancedTable::ThContextMenu: Right now just includes an option for resetting the column width to its initial valueHds::AdvancedTable::ThResizeHandle: A div that acts as a border handle for click-and-drag resizing on resizableAdvancedTablecolumns.Added dependencies
ember-math-helpers📸 Screenshots
🔗 External links
Jira ticket: HDS-4585
Figma file: Figma
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.