Skip to content

Conversation

@majornista
Copy link
Collaborator

@majornista majornista commented Oct 13, 2021

Closes #2449

✅ Pull Request Checklist:

📝 Test Instructions:

  1. Open https://reactspectrum.blob.core.windows.net/reactspectrum/973b3aaf0fbb9bd7407c220817dc5b899e8f7952/storybook/index.html?path=/story/actionbar--default
  2. Keyboard navigate to an item, or the checkbox for the item, in the TableView.
  3. Press Space key to select the item within the TableView, which should expand the ActionBar.
  4. Press Tab key to navigate to first action button within the ActionBar toolbar.
  5. Press ArrowRight key to move focus to the "More Actions" menu button.
  6. Press Space key to expand the "More Actions" menu.
  7. Press Esc key to close the "More Actions" menu.
  8. Focus should restore to the "More Actions" menu button without clearing the TableView selection.
  9. Press Esc key again to clear selections and dismiss the ActionBar.
  10. Focus should restore to the item, or the checkbox or cell within the item, in the TableView where focus left off.

It may be useful testing the following behavior using a tool such as NerdeFocus to log items as they receive focus.

  1. Press Space key to select the item within the TableView again, which should expand the ActionBar.
  2. Scroll the selected item out of view within the TableView body.
  3. Because the collection is virtualized, the focus will shift the the TableView itself.
  4. Press Tab key to navigate to first action button within the ActionBar toolbar.
  5. Press ArrowRight key to move focus to the "More Actions" menu button.
  6. Press Space key to expand the "More Actions" menu.
  7. Press Esc key to close the "More Actions" menu.
  8. Focus should restore to the "More Actions" menu button without clearing the TableView selection.
  9. Press Esc key again to clear selections and dismiss the ActionBar.
  10. Focus should restore to the item, or the checkbox or cell within the item, in the TableView where focus left off, but in order to do so, the TableView will receive focus first, the scroll the appropriate virtualized item into view and then move focus to it.

🧢 Your Project:

Adobe/Accessibility

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

majornista pushed a commit that referenced this pull request Oct 15, 2021
@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@majornista majornista requested a review from LFDanLu October 28, 2021 20:18
@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

Copy link
Member

@ktabors ktabors left a comment

Choose a reason for hiding this comment

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

Can we run chromatic on this change?

I also experimented with selecting a couple rows, moving focus to a cell in an unselected row, going into the ActionBar, pressing escape twice and the focus goes to the cell where the last row selection happened, not where focus was within the table.

@adobe-bot

This comment was marked as outdated.

@snowystinger
Copy link
Member

@ktabors I think the behavior you're seeing makes sense, if you leave a collection that has selections, even if you looked at a few more items, when you come back, you'll be at either the first or last selected item (depending if you arrived via Tab vs shift+Tab). So even though you cleared the selection with Esc, I think returning to where you had a selection makes sense

@majornista majornista changed the title fix(#2449): ActionBar: Esc key to close ActionGroupMenu within ActionBar clears selection and closes ActionBar fix(#2449): ActionBar: Restore focus to SelectableCollection on Esc Apr 4, 2022
@adobe-bot

This comment was marked as outdated.

By using `useLayoutEffect` instead of `useEffect`, we seem to be able to eliminate need to wait two `requestAnimationFrame` calls before trying to restore focus to the `restoreFocusRef.current`.
@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot

This comment was marked as outdated.

@adobe-bot
Copy link

@adobe-bot
Copy link

@adobe-bot
Copy link

@snowystinger snowystinger mentioned this pull request Aug 4, 2022
5 tasks
@adobe-bot
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ActionBar: Esc key to close ActionGroupMenu within ActionBar clears selection and closes ActionBar

6 participants