Skip to content
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

Implications of "Alt+R" reset all keyboard shortcut #874

Open
zepumph opened this issue Oct 8, 2024 · 1 comment
Open

Implications of "Alt+R" reset all keyboard shortcut #874

zepumph opened this issue Oct 8, 2024 · 1 comment

Comments

@zepumph
Copy link
Member

zepumph commented Oct 8, 2024

Over in #789 @jessegreenberg and @terracoda worked out a new keyboard shortcut that resets all. I love it! Very smooth and nice.

Over in buoyancy, @samreid and I found that it opened up new, potentially less-tested features of the scenery keyboard and highlight system that may be worth noting in an issue. In this issue, I came to realize that this is the first time that we could have focus persist through a reset (on something that isn't the button). Buoyancy uses some dynamic elements that are added/removed from the PDOM according to what shape you want to see, and this complexity triggered some bugs and some weird behavior with this shortcut. Some items to share (in no particular order) that may be worth discussion. I'm not sure how we would change things, but I wanted to mention them.

  1. We didn't find these bugs until RC testing because I didn't know about this shortcut and wasn't testing with it.
  2. Resetting while focused on a Node that gets hidden by resetting. What should we do in this case? Based on implementation details in Buoyancy, SOMETIMES it ends up blurring everything such that the next tab is at the very top, while other times focus moves to the next available item, resulting in focus on a semi-random control-panel item after reset all. Given the general best-practice of not swapping focus out from under a user, it isn't clear which of these two is actually better.

To play around with this, I'd use the 4th screen of Buoyancy: Shapes. You can get different behavior in three different situations:

  1. Tab to the default initial block, grab it, move it, and then reset all. The focus and highlight will still be on this block (since visibility of that block doesn't change) Yay!
  2. Switch the shape of the default block, then grab it, move it, and reset all. Focus will blur, but the next tab will focus that block (first thing from the top).
  3. Switch scenes to add the second block, repeat step (2) and see that after reset, focus is in the middle of the scene graph.

Please note that this does not block buoyancy publication, it is just something that came out of testing that I wanted to share with the team.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 8, 2024

Ah, the problem makes sense, thanks.

We didn't find these bugs until RC testing because I didn't know about this shortcut and wasn't testing with it

I am not sure how to best solve. Maybe we can create a checklist of features to make sure things are verified during dev tests.

Given the general best-practice of not swapping focus out from under a user, it isn't clear which of these two is actually better.

Usually, if the reset action removes a component, I think it is fine for the browser to do its default behavior. The user will understand since they initiated the reset.

I can see how it is a little clunky in this buoyancy example though. ResetAllButton could provide a special callback option for focus management. Or the reset listener in buoyancy could implement this.

I thought about some general solutions like

  • Move focus to the reset all button (bad)
  • Always blur the focused element (bad)
  • Try to restore focus to the current location in the DOM by looking at the previous focusable when reset starts and then focusing the element after that. (behavior will be unpredictable/complicated/better off without).

I think for now, we should leave as is.

@jessegreenberg jessegreenberg removed their assignment Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants