-
Notifications
You must be signed in to change notification settings - Fork 12
feat(DataView): add support for resizable columns #512
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
Conversation
…k observer for jest env
if (key === 'Escape' || key === 'Enter') { | ||
onResize && onResize(e, currWidth); | ||
} |
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.
Is the intent with the keyboard interaction that you'd have to press Enter/Escape to "exit" resize mode, or toggle it on and off? If that's the case, we should ensure either the button has a dynamic accessible name ("Resize" and "Stop resizing"), or has the aria-pressed attribute with a static name.
Also, right now, the button in the new example traps focus on itself with keyboard. I can move away with VoiceOver, but focus still remains on the button (VO focus navigates fine until I turn VO off, where I'm back on the button again).
We may want to include the Tab and Space keys here as well to "exit" or toggle resize mode and call this callback. They could both replace the Escape key, as since this would semantically be a button I'm not sure if "Escape" would be a key to use for toggling the button state.
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.
Will work on the button naming and will add the Tab/Space as exits (and remove Escape).
Would any of these updates also be something we want in Drawer? Since the logic for keyboard navigation is pretty close.
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.
The logic should probably align with one another to a degree... The thing with Drawer is that you're able to tab to the resizable splitter and start resizing immediately, but you don't get the onResize being called unless you press Escape or Enter - which I'm not sure if that's obvious to a user if they don't have to press Enter in order to "activate" resizing.
As an end result (maybe not necessarily anything we resolve right this moment):
-
Resizing should occur on any Arrow press (what Drawer does currently - Left/Down arrows and Right/Up arrows do the same respective resizing)
-
Align on whether we want Enter (and Space if the semantic element is a button as is in this PR) to toggle whether "resize mode" is active or not. Having onResize only called for mouseup/touchend events makes sense since you have to actually click on the resize control to begin resizing and letting go is a natural event to occur when you're done - keyboard is trickier. We could argue that having focus on the resize control and pressing an arrow key is all a user should need to do, though that may beg the question as to when to call onResize (after every arrow press may be a lot, maybe debouncing that as well could be a middle ground).
If we do want to have mechanisms where you have to toggle the resizing on and/or off, we'd need to also make that clearer visually. Adding some more accessible text to the name or something can help ("Resize [widget name]" and "Stop resizing [widget name]" for a drawer separator or the resize button here, but we'd need to make that clear for sighted users as well. This may also depend on if there's been any research on the topic before/whether it's a common pattern to have to "enable" resizing on these sorts of things via keyboard. Though our drag/drop stuff does require you to toggle the dragging, so that could be a point for doing the same thing for resizing.
All this said, it may not be as straight forward a topic to deal with at this very moment, in which case we could say maybe it makes sense to follow more what Drawer is doing and aligning both/all of our resizing implementations when we can make more wide sweeping changes to code.
…ft key behavior, refactor logic so default col widths are set for all columns in resizable table
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.
Latest updates look great! Had a few more small comments below.
For context for future-us, Katie and I had briefly talked about it on call yesterday, but the rationale for providing SR text to convey the column width amounts is for an assumption of users who may be utilizing AT like VoiceOver, but not have total loss of sight. The SR text is intended to provide confirmation to these users that the resize action was successful, if they aren't able to easily tell the column was resized visually (but are able to make out other details in the table, or in a case where maybe the increment prop is so low the change in size isn't immediately obvious)
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.
Temporary LGTM! I'll take another look once icon/styling updates are pushed
558e21c
to
f89d922
Compare
… examples, update example resize callback with id
…t exception by using Fragment
🎉 This PR is included in version 6.4.0-prerelease.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
closes #507
Adds support for resizing columns, including with keyboard, touch, and RTL support.
Passing
isResizable
to theDataViewTable
(enables the table scrolling) and aresizableProps
object withisResizable
set to true to each header cell will enable resizing.The following options are available for resizing:
onResize
- callback which will pass back the updated width after resizewidth
- sets the default column width (updated by resizing) to allow persisting resized columnsminWidth
- sets a minimum width that a column is allowed to shrink toincrement
- sets the pixel interval that arrow keys will shift a column width for keyboard navigationAssisted by AI or the IntersectionObserver setup (Goggle) and test fixes/updates (Cursor).