Conversation
|
Very nice!! 👏 Some minor comments:
|
Sure, I feel the same. An alternative is to adjust the widths only on init and table resizing. I think it would be good enough and less weird. |
OK! A slightly more elaborate solution (too much maybe) would be to avoid the right border if there are more columns. ie if there are 10 columns, and 7 are within the viewport, the 7th column cannot end exactly on the table border, but we would have at least 20px shift right or left. |
|
I experimented with the sizing a bit and overall I think this behavior is pretty good. One thing that would be nice would be smart fitting column width on double click, similar to how it works in sheets/excel. If it could be reduced to the minimum needed to contain the values, which would be especially useful for columns like ids or numbers. |
Do you think we should do it at init? When the total width of all columns is less than the available space, I currently add space evenly to all the columns. An alternative, which I don't like because it's less balanced, is to add all the space to the largest column. |
|
I'll try to change the rules as follows:
|
it fixes the types. Also: it removes the need to fix the value on the first load (isInitialized state)
4b850f6 to
e6f4486
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the previous fixed‐width hook with a more flexible useColumnStates solution, updates how column widths are measured, persisted, and adjusted to fit available space, and refactors related components to use the new API.
- Introduces
useColumnStatesand generic parse/stringify inuseLocalStorageState - Implements dynamic width adjustment algorithm in
helpers/width.tsand connects it toHighTableviasetAvailableWidth - Updates
ColumnHeader,ColumnResizer,TableCorner, and tests to use the new state shape and show fixed-width indicators
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/useLocalStorageState.ts | Added generic parse/stringify options and simplified save logic |
| src/hooks/useColumnStates.tsx | New hook managing measured vs. fixed widths and storage |
| src/helpers/width.ts | New measurement helpers and adjustMeasuredWidths logic |
| src/components/HighTable/HighTable.tsx | Wired in useColumnStates, report available width |
| src/components/ColumnHeader | Switched to useColumnStates, toggling fixed/auto resize |
| src/components/ColumnResizer | Updated to use forceWidth/autoResize and new pointer state |
| src/components/TableCorner | Added forwarded ref for measurement |
| src/components/HighTable/*.test.tsx | Updated tests to expect new storage suffix and state shape |
Comments suppressed due to low confidence (3)
src/components/ColumnResizer/ColumnResizer.tsx:102
- The handling for the 'Escape' key has been removed, so users can't cancel a resize operation via keyboard. Reintroduce
if (e.key === 'Escape') { setActiveKeyboard(false); navigateToCell?.(); return; }.
const onKeyDown = useCallback((e: KeyboardEvent) => {
src/components/ColumnResizer/ColumnResizer.tsx:134
- For better ARIA slider semantics, add
aria-valuemin={minWidth}andaria-valuemax={/* some max */}so assistive tech knows the allowed range.
aria-valuenow={width}
src/components/ColumnResizer/ColumnResizer.tsx:102
- No existing tests cover 'Escape' key behavior in the resizer. Consider adding a unit test to verify that ESC cancels resizing and returns focus to the parent cell.
const onKeyDown = useCallback((e: KeyboardEvent) => {
|
re low confidence Copilot comment about aria-valuemin, see: #202 the two other comments are wrong |
|
Merging. We can fix things later if needed. Thanks for the comments, it's better now! |
|
Awesome great work @severo!! It was really annoying me having to resize columns all the time, this is a huge improvement! |
fixes #159
Some rules:
minWidth(hardcoded to 50px).A demo:
Screencast.From.2025-05-28.17-02-15.mp4