Skip to content

Updating column resize to support mode where resizer is always visible#4077

Merged
LFDanLu merged 29 commits into
mainfrom
updates_to_column_resizing
Mar 17, 2023
Merged

Updating column resize to support mode where resizer is always visible#4077
LFDanLu merged 29 commits into
mainfrom
updates_to_column_resizing

Conversation

@LFDanLu
Copy link
Copy Markdown
Member

@LFDanLu LFDanLu commented Feb 16, 2023

split out from #4061 since we need these changes to go in before the docs changes, see that PR for more details and alternative approaches

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

RSP

Comment on lines -206 to -211
onFocus: () => {
// useMove calls onMoveStart for every keypress, but we want resize start to only be called when we start resize mode
// call instead during focus and blur
startResize(item);
state.tableState.setKeyboardNavigationDisabled(true);
},
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes made in this PR mean that onResizeStart/End aren't triggered unless a resize operation actually happens. Entering and leaving resize mode without changing the size doesn't cause those event to fire.

Open to opinions on this change. Note that If we want to preserve this behavior then we'll need a new prop like in a691280 so we can start resizing when the resizer is focused by some external action

@rspbot
Copy link
Copy Markdown

rspbot commented Feb 16, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Feb 16, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Feb 16, 2023

onBlur: collectionProps.onBlur,
onFocus
}), [onFocus, collectionProps.onBlur]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was were running into a case where calling state.setKeyboardNavigationDisabled(true); in TableView from the resize menu item would break grid keyboard navigation due to the timing with which the listeners from useSelectableCollection were turned off by isKeyboardNavigationDisabled. If we immediately turned off keyboard navigation before focus moved from clicking the column's menu item to the resizer, this line wouldn't be called and thus useSelectableItem wouldn't focus the table cell as you tried to keyboard navigate

manager.isFocused tracking didn't feel like it should be disabled just because keyboard navigation was disabled thus the addition of the navDisabledHandlers

Copy link
Copy Markdown
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets make the double grip resizers single when not resizing, it is hard to tell that I've entered resizing

stories need some actions so i know when resizing started/ended

There's an issue on the RSP component side, the draggable resizer disappears on mouse down and doesn't reappear until you move the mouse

Comment thread packages/@react-aria/table/stories/useTable.stories.tsx Outdated
get rid of inline styles and fix case where there isnt a separate trigger for starting column resize
this unfortunately causes a difference in behavior between starting a drag on menu (no resizestart until move) and starting a drag via mouse/touch (resizestart immediately on press)
test would blur on rerender causing a column width update even though resizing wasnt happening. Changed conditonal so calling endResize only causes value updates if we are resizing
@rspbot
Copy link
Copy Markdown

rspbot commented Feb 17, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Feb 17, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Feb 17, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Feb 17, 2023

remove selection from example to mirror docs
@LFDanLu LFDanLu mentioned this pull request Feb 17, 2023
5 tasks
@rspbot
Copy link
Copy Markdown

rspbot commented Feb 17, 2023

this is for the aria table example where resizing is entered manually via Enter while focused on the resizer
@rspbot
Copy link
Copy Markdown

rspbot commented Feb 17, 2023

Comment thread packages/@react-aria/table/src/useTableColumnResize.ts Outdated
@rspbot
Copy link
Copy Markdown

rspbot commented Feb 18, 2023

ktabors
ktabors previously approved these changes Feb 22, 2023
Comment thread packages/@react-aria/grid/src/useGrid.ts Outdated
Comment thread packages/@react-aria/table/stories/example-docs.tsx Outdated
Comment thread packages/@react-aria/table/stories/docs-example.css
if the user enters the table using control+option+arrow keys in voiceover, they will be virtual modality so we want the description for press enter to resize to be audible there
@rspbot
Copy link
Copy Markdown

rspbot commented Mar 1, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 1, 2023

margin applied on the visually hidden input makes scrollIntoView think it needs to scroll it
@rspbot
Copy link
Copy Markdown

rspbot commented Mar 1, 2023

Comment thread packages/@react-aria/table/stories/docs-example.css Outdated
Comment thread packages/@react-aria/table/src/useTableColumnResize.ts Outdated
Comment thread packages/@react-aria/table/stories/docs-example.css Outdated
@rspbot
Copy link
Copy Markdown

rspbot commented Mar 3, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 3, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 3, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 7, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 17, 2023

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 17, 2023

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }

@LFDanLu LFDanLu merged commit be7ecfe into main Mar 17, 2023
@LFDanLu LFDanLu deleted the updates_to_column_resizing branch March 17, 2023 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants