-
Notifications
You must be signed in to change notification settings - Fork 49
AdvancedTable - Column resizing #2849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
79a538c
to
9bd84b3
Compare
d243eb8
to
ccd8c7b
Compare
packages/components/src/components/hds/advanced-table/th-context-menu.hbs
Outdated
Show resolved
Hide resolved
packages/components/src/components/hds/advanced-table/th-resize-handle.hbs
Outdated
Show resolved
Hide resolved
packages/components/src/components/hds/advanced-table/th-resize-handle.ts
Outdated
Show resolved
Hide resolved
packages/components/src/components/hds/advanced-table/th-resize-handle.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
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 adds column-resizing capabilities to the AdvancedTable component.
- Introduces a
column
model to track width, min/max, and restore behavior - Adds
ThResizeHandle
andThContextMenu
components for drag/keyboard resize and context actions - Updates table and header templates, styles, and table model to wire up resizing and width persistence
Reviewed Changes
Copilot reviewed 23 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
showcase/tests/unit/.../column-test.js | Adds tests for default and pixel-width behaviors |
showcase/app/templates/components/advanced-table.hbs | Demonstrates new resizable-column examples |
showcase/app/styles/.../advanced-table.scss | Utility classes for truncation and cell layout |
showcase/app/controllers/.../advanced-table.js | Defines demo column sets and noop resize action |
packages/components/src/styles/components/advanced-table.scss | Styling for resize handles, context menus |
packages/components/src/components/hds/advanced-table/types.ts | Adds minWidth /maxWidth bindings to column API |
packages/components/src/components/hds/advanced-table/th.ts | Wiring for context menu and resize handle |
packages/components/src/components/hds/advanced-table/th-sort.ts | Similar wiring for sortable headers |
packages/components/src/components/hds/advanced-table/th-resize-handle.ts | Core drag-and-keyboard resize logic |
packages/components/src/components/hds/advanced-table/th-context-menu.ts | Context-menu options for reset/resize |
packages/components/src/components/hds/advanced-table/models/table.ts | Table model updated to support resizable columns |
packages/components/src/components/hds/advanced-table/models/column.ts | New column-tracking class with width logic |
packages/components/src/components/hds/advanced-table/index.ts | Component logic refactored to delegate to model |
packages/components/src/components/hds/advanced-table/index.hbs | Template updated to use new model and modifiers |
packages/components/package.json | Added ember-math-helpers dependency |
.changeset/chilly-cheetahs-rescue.md | Changelog entry for minor release |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (4)
showcase/tests/unit/components/hds/advanced-table/models/column-test.js:227
- The unit tests cover width conversions and restoration, but lack test cases for
onPreviousColumnWidthRestored
andonNextColumnWidthRestored
actions. Consider adding tests to ensure those methods adjust the widths correctly.
});
packages/components/src/components/hds/advanced-table/models/table.ts:100
- [nitpick] When using a custom
sortingFunction
, the code callsrows.sort(criteria)
directly on row objects. If the sorting function expects cell values rather than row objects, this may misbehave. Consider invoking the custom function with the intended column values (e.g.,criteria(a[columnKey], b[columnKey])
).
if (typeof criteria === 'function') {
packages/components/src/components/hds/advanced-table/index.ts:224
- The constructor in
HdsAdvancedTable
appears to have mismatched closing braces after adding the resizable columns assertion, which may lead to a syntax error. Verify and restore the correct closing braces for both theif
block and the constructor.
});
packages/components/src/components/hds/advanced-table/th.hbs:68
- [nitpick] The logic for rendering the context menu and resize handle is duplicated in both
th.hbs
andth-sort.hbs
. Extract this into a shared partial or component to reduce template duplication.
{{#if @column}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!! Looking slick.
📌 Summary
If merged, this PR will allow users to resize AdvancedTable columns.
🛠️ Detailed description
column
model alongside the existingtable
androw
models.@hasResizableColumns
(boolean) to the index component API. This controls whether columns are resizable@onColumnResize
(function) argument to the index component. This function is called with column resize details when a column is resized.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 resizableAdvancedTable
columns.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.