Make sure help panel is focused when open, and focus is restored when closed

[acelaya]

This PR requires hypothesis/frontend-shared#941

This PR leverages the new Dialog component capabilities to be focused when opened, and restore focus when closed, to enhance the help panel.

There's one limitation though. The help panel is currently wrapped in a Slider, which adds a nice slide-down/slide-up animation when opened/closed.

This makes the initialFocus="auto" in the Dialog to not work as expected.

As a POC I have removed the slider, just to verify that we can fix the accessibility issue with the new Dialog. However, if we want to keep the animation, the Dialog needs to be evolved.

You can see the new behavior here.

dialog-help-panel.mp4

[lyzadanger]

@acelaya Thanks for working on this! I think we have a few options here, which can be briefly summarized as:

• Make changes to the Slider component, OR
• Make changes to the Dialog component, OR
• Don't use a transition

Let's assume we still want a slide transition, so we'll strike the third option from the list.

A quick(er) way to solve this would be to alter the Slider component such that it takes a callback for when the slide transition is complete. There's already an onTransitionEnd handler in Slider, so adding this wouldn't be too much work. Then you could opt out of Dialog's initialFocus (set it to manual), pass a callback to Slider, and set focus in that callback. This, of course, sort of loses the impact of changes to Dialog for focus routing, but it's one option to get this done quickly.

A more systematic and thorough approach might be to make transitions a concern of Dialog. MaterialUI has an intriguing way of dealing with this that might be something to crib from. In their component collection, relevant components accept a TransitionComponent prop. A you can see from the docs there, a transition component can be any component that conforms to a particular props API. These transition components can be used directly or passed to relevant components as a TransitionComponent prop.

If we implemented a similar model, we could:

• Establish transitions in the shared package, starting with Slider, updating Slider's API to conform both to a simplified "transition component" API and our own API conventions.
• Update Dialog to take a transition component optionally and perform its focus routing at the appropriate time based on that transition component's transition state.

We might be able to take a hybrid approach in which:

• We update Slider in place (in the client) to some subset of a "transition component" API (FWIW, I don't particularly love the prop-naming/API in MUI's transition components and we might not want all of them. I'm especially not won over by the in prop naming). We might want to accept callback props for when the transition has completed in its "open" state and for when the transition has completed in its "exit" state, probably.
• We update the sidebar panels in the client to manually route initial focus after the Slider is complete.
• We simultaneously start working on adding Slider to the shared package and updating Dialog to take a transition-related prop

[acelaya]

Ok, I think I get the idea now. What I'm going to do is:

• Create a branch in frontend-shared, extending the Dialog to optionally accept a transitionComponent prop, and use it as the wrapping component if present, falling back to a Fragment otherwise.
  • This component should accept an onTransitionEnd callback prop and a visible flag. I'll leave onTransitionStart out for now, unless I see it is needed while I'm working on it (we can discuss the naming).
  • The Dialog will use those props to properly focus the component when initialFocus="auto", and restore the focus once closed.
• Extend the Slider component in client to optionally accept onTransitionEnd, to conform with the contract expected by Dialog above.
• Put everything together in client.

In the future/meantime:

• We can think on what are the properties a transitionComponent should have.
• We can decide if we want to move the Slider to frontend-shared.

[acelaya]

I think I have it 🙂

focus-and-transition.mp4

I will create a new PR and close this one, so that I don't have to edit the description and the context of this PR is lost in case we read it later in time.