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

Focus behavior is inconsistent when switching between mouse and keyboard. #750

Open
pixelzoom opened this issue Oct 4, 2021 · 19 comments
Open

Comments

@pixelzoom
Copy link
Contributor

While working on Fourier, I reported that behavior when switching between mouse and keyboard seemed inconsistent and unpredicatable. Sometimes pressing Tab after using the mouse takes you to the next component in the traversal order. Sometimes it takes you back to the beginning of the traversal order.

@jessegreenberg and I paired on this via Zoom. We discovered that ComboBox is responsible for the inconsistent behavior — it sets focus regardless of whether input was from mouse, touch, or keyboard. So if you interact with a ComboBox using the mouse, then press Tab, you’ll go to the next component in the traversal order. Do the same with any other UI component, and pressing Tab takes you back to the beginning of the traversal order.

ComboBox's behavior is actually seems like a better UX, and may also result in a cleaner implementation (no need to check input type before setting focus.) @jessegreenberg will investigate further.

@jessegreenberg
Copy link
Contributor

The source of inconsistency is mostly because of these lines:

            // no need to do this work unless some element in the simulation has focus
            if ( FocusManager.pdomFocusedNode ) {

              // if the event trail doesn't include the focusedNode, clear it - otherwise DOM focus is kept on the
              // active element so that it can remain the target for assistive devices using pointer events
              // on behalf of the user, see https://github.com/phetsims/scenery/issues/1137
              if ( !event.trail.nodes.includes( FocusManager.pdomFocusedNode ) ) {
                FocusManager.pdomFocus = null;
              }
            }

combined with our decision to more liberally focus components without checking if an event came from the PDOM.

@jessegreenberg
Copy link
Contributor

The above code snippet clears focus if a Trail does not include the focused Node, but only does that if there is already focus in the simulation. I think it would be more consistent if we always moved focus to where the pointer is if the Trail includes a Node that is focusable.

// focus should follow the mouse/touch events when possible so that transitioning between mouse/touch and alternative
// input is intuitive
const focusableNode = _.find( event.trail.nodes, node => node.focusable );
if ( focusableNode ) {
  focusableNode.focus();
}
else {
  FocusManager.pdomFocus = null;
}

But I want to think through how this may impact cases where we are already moving focus around with scripting, like ComboBox, GrabDragInteraction, and Dialog. And also how this may impact iOS VoiceOver accessibility.

@jessegreenberg
Copy link
Contributor

The result of #750 (comment) is that focus would generally focus mouse interaction. Then, if you press "Tab", focus would move the next item in the traversal order. In phetsims/vegas#96 @arouinfar mentioned that this was not expected when focus was moved automatically. In that case, focus moved into a dialog that was opened from a mouse press and on the next tab press focus moved to the next item in the traversal order.

We should consider what is expected in this case.

@pixelzoom
Copy link
Contributor Author

This is now blocking phetsims/vegas#96, and therefore blocking phetsims/fourier-making-waves#194 and Fourier 1.0 RC.

@jessegreenberg
Copy link
Contributor

Just an heads up @pixelzoom, I am not likely to work on this until next week. This could be a good general improvement, but I also didn't see confirmation in phetsims/vegas#96 that it was blocking upcoming publications. If that isn't correct please let me know.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 6, 2021

... but I also didn't see confirmation in phetsims/vegas#96 that it was blocking upcoming publications

phetsims/vegas#96 is labeled as blocking, see phetsims/vegas#96 (comment). Any issue reported during RC testing is blocking by default until these's an explicit decision that it's not blocking.

@jessegreenberg
Copy link
Contributor

Many sims with alternative input have been published with the behavior described here and in phetsims/vegas#96 and I don't think we need to hold Fourier for it while we make improvements in this issue. I would verify that phetsims/vegas#96 (comment) is blocking Fourier 1.0.

@pixelzoom
Copy link
Contributor Author

From phetsims/vegas#96 (comment):

Discussed via Zoom with @arouinfar. She's OK publishing Fourier 1.0 with the current behavior of this dialog. So removing the "blocking" label.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 7, 2021

This was discussed in a design meeting 10/7/21. We took a look at how Chrome manages the combination of mouse/touch and alternative input in basic HTML examples.

<!DOCTYPE html>
<html>
  <head>
    <meta charset="UTF-8">
    <title>TEST PAGE</title>
  </head>
  <body>

  <button>Press me!</button>
  <button>Press me!</button>
  <button>Press me!</button>
  <button>Press me!</button>
  <button>Press me!</button>
  <button>Press me!</button>

  <input type="range">
  <input type="range">
  <input type="range">
  <input type="range">
  <input type="range">

  <br>
  <br>
  <br>
  <br>
  <button id="open-button">Move to Panel</button>

  <div>
    <button>Close</button>
    <br>
    <button>Return</button>
    <br>
    <button id="next-button">Next Level</button>
  </div>
  </body>

<script>
  const openButton = document.getElementById( 'open-button' );
  const nextButton = document.getElementById( 'next-button' );

  openButton.addEventListener( 'click', () => nextButton.focus() );
</script>
</html>

Chrome always puts focus on the element that receives interaction with mouse or touch. Then, pressing tab will always put focus on the next element in the traversal order. Focus highlights are hidden until the keyboard is used to interact. This behavior made sense to everyone.

For cases where focus is set with script (like when a Dialog or ComboBox opens) it was agreed that pressing tab should still focus the next element in the traversal order. Some thought in this case that the next tab press should put focus on the first focusable element in the popped up context. But after understanding that using by mouse/touch focus actually moves to the component in the popup even if the focus highlight is not shown, putting focus on the next item in the traversal order made sense to everyone.

So we will proceed with making it so that focus follows mouse/touch input while tab always moves to the next element in the traversal order. #750 (comment) is where I would start for the implementation. We expect it to be complicated when focus is set in this way but also set by an activated component. For example, if you click on a ComboBoxButton focus needs to go to one of the list items and NOT the ComboBoxButton (which is what received input).

We are not working on this right away but will discuss its priority/timeline in an upcoming planning meeting.

@jessegreenberg jessegreenberg removed their assignment Oct 7, 2021
@terracoda
Copy link
Contributor

This all sounds good to me, too.

@jessegreenberg
Copy link
Contributor

We discussed this once before but decided not to work on it, just referencing that issue here: phetsims/scenery#713. I reviewed comments in that thread and they are in line with #750 (comment).

@zepumph
Copy link
Member

zepumph commented Oct 14, 2021

Potentially a subset of phetsims/scenery#1298, depending on what designers think.

@jessegreenberg
Copy link
Contributor

This issue is the right way to resolve phetsims/circuit-construction-kit-common#901 - In that issue, focus gets set in the downstream iframe before mouse input happens in the upstream iframe. Then global keyboard events get sent to the downstream sim instead of the upstream sim. If focus followed the mouse and the upstream sim became active on global key events would go to the frame that most recently received input.

@jessegreenberg
Copy link
Contributor

This came up again in a recent dev test from @KatieWoe. It was confusing that focus started from the beginning after interacting with some components and not others and we remembered it was to be improved someday in this issue.

@matthew-blackman
Copy link

matthew-blackman commented Jan 23, 2024

Notes from 1/23 meeting with @samreid and @jessegreenberg:

I will schedule design meeting time this week so that we can discuss this further.

@terracoda
Copy link
Contributor

I am glad this issue is getting attention. There are some inconsistencies across sims, and looking into this more deeply will be good.

@emily-phet
Copy link

This issue results in inconsistent focus behavior when switching inputs. This is not a blocking issue for any work on my grants, so I leave it up to @kathy-phet to decide how/when to prioritize efforts on this.

@kathy-phet
Copy link

kathy-phet commented Jan 25, 2024

[Notes from design meeting with AR, JG, MK, TS, MB, SR, NS, KP (might have missed someone)] Let's prioritize this for an upcoming iteration - 13 effort point kind of thing, cross compatible

Considerations around the design:
This feels like a new feature, rather than scaling alt input
What is the priority of the value of this mouse/keyboard compatibility (better UX) vs advancing improvements in slider improvements (scale Alt-input to more sims)?

Let's start with creating a project board around alt-input with labels that define them in these 2 categories.

We might be able to capture most cases, but not all cases, with some effort.

Right now with any sim in a iFrame, a student clicks in, it doesn't grab focus.

@emily-phet
Copy link

Most important is alt input in more sims.
I think the it's just a matter of whether or not you want to carry forward into new sims this known issues.

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

9 participants