[Flare] Rethinking Focus

[trueadm]

I think we need to rethink how focus works in React. React Flare is the perfection opportunity to allow us to do this, so here are some of my thoughts. None of these ideas wouldn't be possible if it weren't for the great ideas from @sebmarkbage, @devongovett and @necolas have had. Furthermore, the discussions in #16000, #15848 and #15849 got me thinking on a better system.

Focus is a mess on the DOM, so let's not use the DOM

Focusing on the DOM is a mess today. We couple ideas around ideas around things like tabIndex and whether a specific browser treats something as focusable. This is very much a hard-coded disaster where no one really agrees on a good formula for success. Not to mention, that this just doesn't translate well for a declarative UI. How does one tab to a specific node that isn't focusable? How does one use keyboard arrows to navigate a table using keyboard arrows?

Then there's implementation. Without relying on an attribute on an element or a ref, it's very hard to say: "Hey look, let's focus to this node, given this criteria". Not to mention the performance overhead of doing this: querying or "collecting" focusable elements is an expensive O(n) task, which doesn't scale for large applications well. I noticed that wrapping the an internal large app with <FocusScope> and then collecting all focusable nodes took over 850ms on Android using Chrome. Querying the DOM nodes took even longer.

Lastly, we can't use the DOM with React Native and the story for handling focus with React Flare is important. If we instead had a React system for handling focus, then both the web and RN would be consistent and performant.

Accessible components

We already have the <Focus> and <FocusScope> event components. We could extend on React Flare and introduce a way of layering accessibility logic on to host components. In this I introduce a new API called createAccessibleComponent, but really it could be anything – ignore the naming! This is purely hypothetical discussion for now.

// input is not focusable
<FocusScope>
  <input type="text" placeholder="Enter your username" /> 
</FocusScope>

const FocusableInput = ReactDOM.createAccessibleComponent((props, focusable) => {
  return <input tabIndex={focusable ? 0 : -1} {...props} />;
});

// now it's focusable
<FocusScope>
  <FocusableInput
    type="text"
    placeholder="Enter your username"
    focusable={true}
  />
</FocusScope>

If you don't use a FocusScope, then the normal DOM behaviour will continue to work as expected. FocusScope will only care about these new types of accessible component.

The focus manager should be encapsulated and relative to FocusScope

In order for focus management to be powerful, it needs to be baked into React. Event responders like FocusScope can let the manager know what scope it should be interacting with given a particular <Focus> that focuses occur in. FocusScope will also fully override the browser tabbing behaviour (like it does now) to ensure tabbing works as expected:

import { focusManager } from 'react-events/focus';

focusManager.getFocusedNode();
focusManager.getFocusedId();
focusManager.focusFirst(isRTL?: boolean = false);
focusManager.focusLast(isRTL?: boolean = false);
focusManager.focusPrevious(fromId?: string, isRTL?: boolean = false);
focusManager.focusNext(fromId?: string, isRTL?: boolean = false, );
focusManager.focusById(id: string);

const FocusableDiv = ReactDOM.createAccessibleComponent((props, focusable) => {
  return <div tabIndex={focusable ? 0 : -1} {...props} />;
});

<FocusScope onMount={() => focusManager.focusFirst()}>
  <FocusableDiv focusable={true} />
  <FocusableDiv focusable={true} />
  <div tabIndex={0}>You can't focus this</div>
</FocusScope>

Focusing by focusId will propagate until an focusId is found. So this would matter for cases such:

const FocusableDiv = ReactDOM.createAccessibleComponent((props, focusable, focusId) => {
  return <div tabIndex={focusable ? 0 : -1} {...props} />;
});

<FocusScope>
  <FocusableDiv focusable={true} focusId="focus-me" />
  <FocusScope>
    <FocusableDiv focusable={true} focusId="focus-me" />
  </FocusScope>
</FocusScope>

If focusManager.focusById('focus-me); was used on the inner FocusScope, it would focus the inner button. If used on the outer FocusScope, it would focus the outer button. If the outer FocusScope didn't have an id that matched, then it would propagate the lookup to the inner FocusScope.

Doing this, it makes it possible to apply keyboard navigation:

function handleKeyPress(key) {
  if (isValidArrowKey(key)) {
    const currentId = focusManager.getFocusedId();
    const nextId = findNextId(currentId, key);
    focusManager.focusById(nextId);
  }
}
<FocusScope onKeyPress={handleKeyPress}>
  <FocousableCell focusable={true} focusId="AA" />
  <FocousableCell focusable={true} focusId="AB" />
  <FocousableCell focusable={true} focusId="AC" />
  <FocousableCell focusable={true} focusId="BA" />
  <FocousableCell focusable={true} focusId="BB" />
  <FocousableCell focusable={true} focusId="BC" />
  <FocousableCell focusable={true} focusId="CA" />
  <FocousableCell focusable={true} focusId="CB" />
  <FocousableCell focusable={true} focusId="CC" />
</FocusScope>

Furthermore, <FocusScope>s can also have focusIds that allows you to move focus to a specific scope. That particular event component can then act upon receiving focus <FocusScope onFocus={...}>.

It can simplify <Focus>

<Focus onFocus={...}>
  <div>
    <FocusableDiv focusable={true} />
  </div>
</Focus>

Before, focus would only be of the direct child of the <Focus> component. This made it somewhat problematic when you wanted to find the focusable element that was not a direct child. Focus no longer needs to be coupled with "bubbling up" through the DOM, but rather it bubbles from accessible component to event components. So doing this, will still result in the nearest focusable child being passed to the Focus:

<Focus onFocus={...}>
  <div>
    <FocusableDiv focusable={true}>
      <FocusableDiv focusable={true} />
    </FocusableDiv>
  </div>
</Focus>

This can be fast too

In terms of performance, we can actually fast-path how this all works because we're no longer using the DOM, but event components within the Flare event system. We'd have optimized data collections that ensure that the least possible time is taken traversing focusable elements by leveraging a separate internal data structure that is separate from the Fiber and DOM structures. The cost is that this will take additional memory and time to construct when a focus scope gets mounted/unmounted. Given this shouldn't be a rapid event, it's worth the trade-off.

Also, given we're not wrapping FocusScope with a context provider (as mentioned in the FocusManager PR), which should get improved performance from not needing to do context lookups and traversals.

Focus and FocusScope, focusManager

Given that they now share underlying implementation details, they all should come from the same module. So going forward, it makes sense to import them all form react-events/focus.

The nice benefit from this is that this actually fixes a bunch of issues with the current implementation, where we can't use FocusScope as a hooked event component. With the changes outlined in this issue, it should allow for them to be used via the useEvent hook.

We can build in great dev tooling around the focus system

We can build in great support for debugging in React Dev Tools when working with focus and
this will help improve accessibility within apps that use <Focus>, <FocusScope> and focusManager. Plus it would support any future APIs that add accessibility benefits to components.

[eps1lon]

It something is "interactive", thus focusable, it should always be tabbable too.

This caught my eye but only because I'm doing keyboard navigation on lists wrong currently. Anyone thinking this is bad for long lists which are navigateable by arrow keys: The whole list stays focused while listitems are marked with aria-activedescendant. Focus doesn't move to listitems. See WAI-ARIA authoring practices

The rest looks fine. Should this be an RFC?

[trueadm]

@eps1lon That’s a good point. How do you envisage focus targets working in your use case? Note also: controlling focus directly should work in tandem with aria properties.

The rest looks fine. Should this be an RFC?

This is for React Flare which is still all experimental right now (and internal only).

[eps1lon]

I don't think I would use these components for a list that is navigateable with the keyboard. Unless the examples on the WAI-ARIA authoring practices are inaccessible it seems like aria-activedescendant works good enough. At least NVDA works reasonably well and doesn't require actual focus on the selected options.

It seems like "just" adding tabIndex=-1 to the interactive widgets is good enough. The problem only exists if we want to implement roving tabIndex.

FocusScope seems only interesting for Modals as far as I can tell. But I've only glanced at the current implementation progress and FocusScope rfc so I'm probably missing more use cases.

[trueadm]

@eps1lon FocusScope is far more about modals, it's also for things like dropwdowns, lists, or anywhere you'd want to control focus explicitly (like keyboard navigation). In fact, looking at the WAI-ARIA spec and aria-activedescendant – you could do the exact same with FocusScope outlined in this issue without needing aria-activedescendant whilst remaining accessibility compliant – unless I'm missing something.

When you use the keyboard arrows to move up and down the list, you'd move current focus between accessibility components. The additional benefit is this will work in browsers that don't support aria-activedescendant. It would also translate nicely to a world where we have lists in React Native that use keyboard navigation.

To clarify though – I don't think you'd be using FocusScope directly in your components. You'd encapsulate it with the component whose responsibility is to have that behaviour. For example <MyAppDropdownList> or <MyAppDialog> etc.

[philipp-spiess]

We have an interesting use case that this could solve if we allow users to bring their custom findNextId method like you outlined above. Our use case requires us to use the DOM order to get the stacking we need (we can't use z-index for various reasons) while being able to use a different tab order.

Would this be something that this proposal can help us with?

[trueadm]

Would this be something that this proposal can help us with?

This would definitely work in those cases. Did you mean z-index or tabIndex btw?

[philipp-spiess]

Did you mean z-index or tabIndex btw?

I did mean z-index. We could use the DOM order for the tab index and define z-index for the render order. The problem is that we'd loose the ability to apply blend modes since all items will create their own stacking context.

We could use tabIndex but this is very hard to get right. E.g. multiple scopes can exist in parallel so we would need to do a lot of bookkeeping to make it work. If tabIndex only works within a <FocusScope/> I think this would be a very good solution as well.

[eps1lon]

you could do the exact same with FocusScope outlined in this issue without needing aria-activedescendant whilst remaining accessibility compliant – unless I'm missing something.

It sounds like you also wanted to enforce that focusable means tabbable which elements of a dropdown shouldn't be. This was my concern.

[trueadm]

I've removed the ideas around FocusTarget above based off of internal feedback. Instead opting for a new type of "accessible component". Let me know your thoughts!

It sounds like you also wanted to enforce that focusable means tabbable which elements of a dropdown shouldn't be. This was my concern.

I've taken your feedback and revised my original post! :)

Notably, instead of using FocusTarget, we now make a (hypothetical) new type of accessibility component that React can optimize for focus operations:

const FocusableInput = ReactDOM.createAccessibleComponent((props, focusable) => {
  return <input tabIndex={focusable ? 0 : -1} {...props} />;
});

// now it's focusable
<FocusScope>
  <FocusableInput
    type="text"
    placeholder="Enter your username"
    focusable={true}
  />
</FocusScope>

[eps1lon]

Maybe I'm thinking to close to how the DOM works but a listitem shouldn't actually be focused i.e. document.activeElementshould never point to it. TheactiveElementshould be the list on which a ArrowDown results in the next listitem being selected. It wouldn't make sense to declare the listitem as the active element because the whole widget is currently active. It's also relevant with regard to styling. Some users might want to apply custom styles with:focus`.

[trueadm]

@eps1lon Yeah, I think it's too close to thinking in relation to the DOM. This new event system allows us to forget about how the DOM and browsers work in regards to much of the complications today. We can set a new set of standards that are more applicable to how applications in React are used and interacted with today – just like React did for UIs.

With Flare, internally we've advised that no one uses pseudo selectors like :focus with Flare – for a number of reasons: it's sync, which doesn't play nicely with React's concurrent rendering and it breaks interfaces that use touch events and also can cause.

With Flare, users wouldn't be using activeElement but rather opting into React's focus system instead using the focus manager. This means that the browser won't be controlling what tab goes to, but instead React will and because of that it can apply custom behaviour when it comes to handling lists with keyboard control and tabbing – which is also fully compliant with accessibility tooling from my testing.

[giuseppeg]

@trueadm I guess @eps1lon's concern might be valid. I will try to clarify and you or him can correct me if I am wrong.

In the case of dropdown menus the list never gets focus. Instead the toggle button is always the active element.

When using the screen reader you can use the up/down arrow keys to navigate the list but (important) focus stays on the toggle button (basically you only control the screen reader cursor with up/down). This is to allow the user to tab away to the next item and close the menu on blur (or at least this is how many implement it).

[trueadm]

@giuseppeg I totally get that. I was saying that the support for that aria property is limited between browsers though and FocusScope can do that behaviour without the need to use it:

function DropdownList() {
  const [showDropdown, updateShowDropdown] = useState(false);

  useLayoutEffect(() => {
    if (showDropdown) {
      focusManager.focusId('dropdown');
    }
  });

  <Focus onFocus={() => updateShowDropdown(true)}>
    <DropdownButton />
    showDropdown && (
      <FocusScope focusId="dropdown" onKeyPress={key => {
        if (key === 'Tab') {
          // Move to the next focusable node outside this focus scope
          focusManager.focusNext('dropdown');
        } else if (key === 'ArrowDown') {
          focusManager.focusNext();
        } else if (key === 'ArrowUp') {
          focusManager.focusPrevious();
        }
      }}>
        <ul>
          <li>Item 1</li>
          <li>Item 2</li>
          <li>Item 3</li>
        </ul>
      </FocusScope>
    )
  </Focus>
}

That's a rough example of what I meant. :)

[eps1lon]

I would guess that you implement it in a way that TAB exits the lists focus scope.

Aside: the focus is on the list not the button that opened it.

[devongovett]

This seems more similar to the original RFC I wrote than the current implementation, and in fact quite similar in terms of the suggested implementation strategy from the RFC.

In terms of perf, the RFC also proposed a separate focus tree similar to what you're describing. Elements are added to the nearest FocusScope as they are mounted, rather than collecting them every time the tab key is pressed.

It also goes back to the singleton focus manager API that I originally proposed, which I think might be easier to use and probably faster than the useFocusManager hook. It allows using it from within the component that rendered the focus scope, and doesn't rely on context.

I had also considered some kind of wrapper to mark focusable nodes, similar to your createAccessibleComponent or use of <Focus> components. See this comment from a previous thread about that. In fact, I prototyped this completely in userland by using a tree of contexts to track the focusable elements.

I originally decided not to go that way due to interoperability concerns. For us, we'd use <FocusScope> as part of a <Dialog> component for example. We cannot control what people render inside that dialog, or force them to wrap their existing DOM elements inside a wrapper. However, this does seem to be what people are doing with design systems/component libraries anyway. Not many people will be using straight up <input> without some kind of wrapper. So this might be ok.

So in summary, this seems pretty good to me. I'm not sure about the name createAccessibleComponent, but that can be bikeshedded. 👍

[trueadm]

@eps1lon yeah I have bugs in that example above, but hopefully it conveys what I was trying to explain better!

[eps1lon]

@eps1lon yeah I have bugs in that example above, but hopefully it conveys what I was trying to explain better!

I was on mobile and didn't see the implementation. This was addressed at @giuseppeg.

But this looks really nice. Especially decoupling it from element ids is pretty nice. No need for some custom id generation logic since this will all be contained within a sub-tree.

[marcysutton]

This sounds awesome; really cool to read through the ideas here. One thing I want to reiterate is be sure to test often in NVDA/JAWS and Voiceover–ARIA roles and accessible names can make a big impact on whether something will be announced when focused.

We're working on some accessible client-side routing improvements in Gatsby.js and my colleague shared this proposal with me–it seems more useful for modals and components than route changes per se, but I'll be following the progress. Let me know if you need any more testing done, I'm happy to try some things out and provide feedback.

[nathanhammond]

Targeting

If the outer FocusScope didn't have an id that matched, then it would propagate the lookup to the inner FocusScope.

I'm not sure I understand the selection algorithm you're proposing:

• First check to see if any of your parents have it, and if so, choose the closest one?
• However, if none of your parents have it, choose from your children based on presumably in-order depth-first DOM traversal? Or maybe only direct-children? Or?

If my understanding is correct, the "choose from your children" feels like it could be dangerous in the event that any of your children rerender and you need to retain DOM synchronicity. If my understanding is incorrect, I'd love to see more explanation on this so I can better understand your proposal.

Ordering

Tab order is defined by tabindex used in combination with positive values. Without something approximating this behavior, focusNext and focusPrevious rely exclusively on DOM order. (This is probably a good thing overall, but constraining the API in a way where it is impossible to have explicit tab ordering is probably not ideal.)

RTL

I'm pretty sure that this is portion of the API is unnecessary. I believe that form elements are by default focused in DOM order (depth-first, in-order), regardless of the RTL/LTR state.

Even if this does end up as something that the FocusManager should keep track of (e.g. because it is required by React Native or I am wrong) it should not be on the user to specify as it can be detected from the context of the elements. (Conveniently skipping over "how to make this fast" in favor of "how to make this ergonomic.")

The only place where I know that it might matter is when clicking into a non-focusable area and deciding which node should be focused next. In the DOM world I'm pretty sure that this still doesn't require knowledge of LTR/RTL as you can simply track the DOM node.

Multiple Roots

In the event of multiple React roots on a page, what should happen between competing FocusManager-using components dealing with mount races? A page can only have one document.activeElement. Without diving into mailing list history, this is presumably why there is not a declarative focus API to begin with: it becomes possible for it to get out of sync.

[trueadm]

If my understanding is correct, the "choose from your children" feels like it could be dangerous in the event that any of your children rerender and you need to retain DOM synchronicity. If my understanding is incorrect, I'd love to see more explanation on this so I can better understand your proposal.

I think (this issue hasn't been updated since), a better solution is to use handles rather than IDs in general. Then we avoid collisions with ids and can avoid all this complicated lookup logic.

I'm pretty sure that this is portion of the API is unnecessary. I believe that form elements are by default focused in DOM order (depth-first, in-order), regardless of the RTL/LTR state.

Last time I checked, RTL did affect focus behaviour for screen readers. With React Flare, we wouldn't use the browser focus system, we use our own internal mechanism (required for Suspense and Portals).

This isn't that big of a concern though, we can drop this from any API if we don't deem it necessary. We don't track DOM nodes though, we track fibers, which have a relative relationship to DOM nodes for host component fibers.

Multiple Roots

We haven't really discussed multiple roots at this time, but I don't see any real issues here. We control focus with the new React Flare system that is independent from the current React event system and also independent from the browser focus system. We can ensure events are properly acted on for their current roots by using the fiber tree to ensure consistency.

[sophieH29]

Hi @trueadm and team, kudos to the work you're doing ❤️ Focus management has been a pain to fully make components accessible.

Based on your example

const FocusableInput = ReactDOM.createAccessibleComponent((props, focusable) => {
  return <input tabIndex={focusable ? 0 : -1} {...props} />;
});

I wouldn't call it createAccessibleComponent, I want to believe that each component has to be accessible by default and it's a much broader topic. Focusability's just ~20% of accessibility.
The other point is that focusable ? 0 : -1, either you set tabindex to 0 or -1, in both cases element will be focusable. In first case (with tabindex=0) it is also tabbable.
So, at end, I see it as

const FocusableInput = ReactDOM.createFocusableComponent((props, tabbable) => {
  return <input tabIndex={tabbable ? 0 : -1} {...props} />;
});

[trueadm]

@sophieH29 The createAccessibleComponent example was never intended to actually be used, it was just a rough example. You're right in terms of what you say about accessibility too, sorry if my above example gave the wrong impression!

[brennanyoung]

I think this discussion need to delve into the notion of screen reader interaction modes.

Nobody has mentioned this yet, but screen readers have two radically different ways of 'focusing' content, depending on whether they are browsing (headings, paragraphs, lists etc.) using a 'virtual cursor', or alternatively 'interacting' (form controls, hyperlinks and other GUI widgets) - which uses the 'tab order' that is perhaps more familiar to those who interact with the web via keyboard.

Focus management which does not take this distinction into account is doomed to failure.

You 'focus' the virtual cursor on non-interactive elements (such as headings) using a keyboard shortcut (e.g. H for next heading, I for next list item), and only then do they get announced.

When you do a 'true' reload, the screen reader will announce the page title and typically start reading the first text nodes - especially those found in H1. When you re-render a single page app, none of this happens by itself. There is no way to programatically set the 'virtual cursor', except via the hack of focusing an element with tabindex set to -1. This is where accessibility implementations collide with the single-page-app paradigm.

[benoitdion]

Looking at this from a react-native point of view this api would make a lot of sense for keyboard navigation/tv remote support. The proposed api has an onFocus callback but I'm guessing it'd also have an onBlur?

[trueadm]

We're no longer exploring the ideas I mentioned in this issue, so closing it now.

[bpierre]

@trueadm is any other solution being explored, regarding focus management?

[trueadm]

@bpierre The focus management stuff was pulled out of React Flare earlier on and the work ended up here (although this is old now, the latest code is only available internally at FB).

https://github.com/facebook/react/tree/master/packages/react-interactions/accessibility/src

[bpierre]

@trueadm Thanks!

[inomdzhon]

@trueadm link is dead :( Or accessibility in another branch?

[trueadm]

Link is dead, we removed that work. You can find it still in history for the react-interactions directory :)

[eps1lon]

@trueadm link is dead :( Or accessibility in another branch?

https://github.com/facebook/react/tree/HEAD@%7B2020-01-10T13:56:31Z%7D/packages/react-interactions/accessibility/src

The refined github extension adds permalinks to any non-permalink github links.