-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Refactor column resizing for controlled state resizing #3672
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
Merged
Merged
Changes from all commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
6adffe8
Refactor column resizing to make a single source of truth
snowystinger dd8512f
Merge branch 'main' into refactor-column-resizing
snowystinger f370934
fix merge
snowystinger 7fd45f9
fix remove column math
snowystinger fdb68a3
remove unused dep
snowystinger 9fe9238
Merge branch 'main' into refactor-column-resizing
snowystinger 9c39b1a
Merge branch 'main' into refactor-column-resizing
snowystinger 3fb4677
Move aria implementation to stately hook instead of layout
snowystinger 69f481d
fix keyboard resizing and default widths
snowystinger a99ca8e
use aria tests for spectrum as well
snowystinger 83c2bd1
rename some props, refactor resizer so it sets on top of everything
snowystinger 36804cb
restore logic to fix tests
snowystinger 0bed81d
fix min/max calculations
snowystinger f39e26b
fix all types and lint
snowystinger 01ada31
Merge branch 'main' into refactor-column-resizing
snowystinger a8ab122
fix import
snowystinger bae3c9e
Merge branch 'refactor-column-resizing' of github.com:adobe/react-spe…
snowystinger af599ac
onResizeEnd called once with sizes
snowystinger d0c5f73
fix aria story missing resize handles
snowystinger 16873d9
Add story descriptions
snowystinger a50b0ec
Fix resizer gets stuck visible
snowystinger 9586a1d
Merge branch 'main' into refactor-column-resizing
snowystinger 2137c2f
simplify column header focus mode
snowystinger ceca13e
clean up and code sharing
snowystinger e9c21b7
fix lint
snowystinger 1f57a19
add test from other PR
snowystinger acbc4c7
add max width as well
snowystinger 35bc3a0
Makes types clearer
snowystinger eb36ff6
clarify min/max width types
snowystinger a04a7a2
fix exports
snowystinger d0acbf7
fix css specificity
snowystinger 0be2489
address resizers getting stuck in hovered state
snowystinger c68d962
Make sure input "value" updates
snowystinger ca8f423
fix lint
snowystinger cf36abc
remove unused api
snowystinger 6ff2ac2
Code reviews
snowystinger f5a26b4
code reviews
snowystinger cd7281a
Merge branch 'main' into refactor-column-resizing
snowystinger 918ddd9
more reviews
snowystinger ad4dadb
Merge branch 'main' into refactor-column-resizing
snowystinger e7d8783
add missed type
snowystinger 62dfe3d
fix types and remove read/write render ref
snowystinger c1b2a75
re-type getDefaultWidth and add strict nulls
snowystinger 67560cb
remove some memos, use existing value for resizer
snowystinger 6f9b0d4
Merge branch 'main' into refactor-column-resizing
snowystinger 2cc7c68
rename confusing variable
snowystinger afb1591
move column layout to table stately
snowystinger a7372f8
add resizing hide header story
snowystinger dff4b4d
move types
snowystinger 9b94c17
Fix onResizeEnd call
snowystinger f330245
Merge branch 'main' into refactor-column-resizing
snowystinger e905d3d
move types
snowystinger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I assume you weren't able to use onMove and onMoveEnd only, hence the addition of the following handlers? Just trying to figure out if the following wouldn't work with onMove/onMoveEnd
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.
if you look at the description for onMove/onMoveEnd it describes their differences. Specifically that they are called at different times and carry the interaction method, allowing the rendering to change in response and do things like change the mouse cursor when using with a mouse, but not during keyboard resizing.
so they are different than onResize. maybe they have a bad name?
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.
Right, I was more wondering if
onMoveStart(which doesn't exist here),onMove, andonMoveEndcould handle the logic fromonResizeStart,onResize, andonResizeEndas well. Now that I type this up, I now realize that the keyboard experience would always fireonMove->onMoveEndper keypress instead of howonResizeEndonly happens when finalizing a resize operation, so I think we are fine hereThere 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.
I'd love to combine them, but I haven't figured out a good way to. Nor have I really figured out a better name for the difference.
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.
TODO: add to followup