Focus Management

[kitsonk]

@kitsonk commented on Wed Nov 16 2016

User Story

A developer needs an easy way to manage focus in an application and easily move that focus.

Discussion

Focus management is a core bit of functionality to "get right". Focus management in a virtual DOM though can require programatically setting focus on the real DOM node during a render. It makes sense to consider an "abstraction" of some sort, that is integrated into the projector, that allows a focus manager to "collect" the real DOM nodes and when a widget needs to have its focus set, it would be a call to the projector with something like setFocus(widget) or setFocus('widget-id'). Possibly this would be an app construct too.

Also, note that in many cases, the DOM/VDom root emitted by the widget would not be the focus node. Therefore we will need a way identify the nodes that are the focus node for a particular widget (e.g. data-focus-node attribute).

---

@mwistrand commented on Tue Apr 04 2017

So would applications be required to pass the projector around in order for widgets to receive focus when they need it? In this case, how might widgets that have specific requirements for keyboard navigation handle focus management without requiring that the application explicitly pass in a projector instance?

---

@kitsonk commented on Wed Apr 05 2017

I think we need to switch it around...

Why would a widget need to grab focus? It might need to express that "if I have focus, this sub part of me should be the focus" and some sort of "if you navigated within me, here is the current ordering of my focus points". As a widget would reveal different sub-parts, it would re-render that "meta-data".

Remember, widgets are reactive. They respond to things. They don't actually do anything if we can avoid it at any cost.

The management part of it would have to occur at a point where some actor has a whole view of everything going on. Widgets shouldn't take focus, otherwise it would be chaos, like little squabbling children.

---

@mwistrand commented on Wed Apr 05 2017

Take a look at the WAI-ARIA menu example. Each menu only has one top-level item with a tabIndex other than -1. As the user presses arrow keys within each item, however, focus is manually placed on that specific item. A property can and should be used to determine whether a similar item in a Dojo 2 menu should receive focus, but if focus management takes place within the projector, how would the two communicate with each other?

---

@kitsonk commented on Wed Apr 05 2017

Exactly the same the widget communicates to the projector now... by invalidating itself and then providing a different render when it is requested.

---

@mwistrand commented on Wed Apr 05 2017

Well, yes, but it's still not clear to me how the widget communicates to the projector (focus manager) that it should receive focus. In other words, invalidate() by itself doesn't mitigate the need to call element.focus() from within onElementUpdated. So how would the focus manager know which of the invalidated widgets (if any) should receive focus?

---

@kitsonk commented on Wed Apr 05 2017

That is my suggestion though that we have a reactive solution to managing focus and abstract the widget developer from the DOM. If that means we have to express a property in our DNodes to affect that, that is what I would recommend.

---

@mwistrand commented on Fri Apr 07 2017

In private conversations the question has come up whether a focus manager is even necessary. My personal view is that having a single controller for setting focus is helpful. While it cannot prevent widgets from fighting over focus like "squabbling children", it does at least provide a vehicle for warning application developers when they are. Otherwise, widget authors would have to wait for bugs to surface before learning whether their widgets are requesting focus unnecessarily. Even something as simple as the following would go a long way to help resolve focus issues:

// list of selectors that can receive focus natively
const tabbable = [ /* ... */ ];

class FocusManager {
	private _focusScheduled = false;

	setFocus(element: HTMLElement) {
		if (this._focusScheduled) {
			console.warn('Focus already scheduled for a different DOM node.');
		}

		const focusable = this._resolveFocusableElement(element);
		if (!focusable) {
			throw new Error('Element must either have a `tabIndex` or be focusable natively.');
		}

		this._focusScheduled = true;
		requestAnimationFrame(() => {
			this._focusScheduled = false;
			focusable.focus();
		});
	}

	private _resolveFocusableElement(element: HTMLElement): HTMLElement | void {
		const tag = element.nodeName.toLowerCase();
		if (tabbable.indexOf(tag) > -1 || element.getAttribute('tabIndex')) {
			return element;
		}

		const focusableChild = element.querySelector('[data-dojo-focus-node]');
		if (focusableChild && focusableChild.getAttribute('tabIndex')) {
			return focusableChild as HTMLElement;
		}
	}
}

---

@kitsonk commented on Fri Apr 07 2017

I would be 👍 for abstracting developers from touching the DOM and 👍 for adding some sort of control point to at least monitor the squabbling children!

---

@smhigley commented on Mon Apr 10 2017

I could see a use for creating a level of abstraction between widget developers and the actual DOM node, but I'm not sure we need to try to apply logic to which element gets focus. In my opinion, focus should never be set except in direct response to a user action, ideally in an event handler. I don't see this interfering with reactive principles, since in my view we shouldn't be keeping track of focus in the first place.

My initial thought for a "focus manager" wasn't to manage focus exactly, but focusability. For elements that should create focus traps (dialogs, flyout menus, anything that visually interferes with the user's ability to interact with part of the page), we need something that will either set tabindex on focusable elements within a container, or that will listen to focus events on document and redirect focus when it leaves the focus trap.

---

@smhigley commented on Fri Apr 21 2017

Playing with Menu, I think I've come up with a pattern for setting focus on child elements that works and isn't buggy (i.e. doesn't ever try to steal focus back when it shouldn't). It involves the parent widget passing the child widget a property that if true, sets focus in onElementUpdated and immediately calls a function also passed in to the child that reverts the focus property to false. So, for example (obviously oversimplified):

// Menu class
private _activeIndex = 0;
private _callFocus = false;

private _onMenuItemFocusCalled() {
	this._callFocus = false;
}

private _onMenuItemKeyDown(event: KeyboardEvent) {
	if (key logic stuff to focus next menu item) {
		this._activeIndex++;
		this._callFocus = true;
		this.invalidate();
	}
}

private _renderChildren(children) {
	return children.map((props, i) => w(MenuItem, {
		(... stuff),
		callFocus: this._callFocus && i === this._activeIndex,
		onKeyDown: this._onMenuItemKeyDown,
		onFocusCalled: this._onMenuItemFocusCalled
	}));
}

// MenuItem class
protected onElementUpdated(element: HTMLElement, key: string) {
	if (key === 'root') {
		const { index, callFocus, onFocusCalled } = this.properties;

		if (callFocus) {
			element.focus();
			onFocusCalled && onFocusCalled(element, index);
		}
	}
}

I'm interested in everyone's thoughts, and ideas about abstracting away the need to include onElementUpdated in the child widget, and the need to correctly handle onFocusCalled in the parent widget.

---

@smhigley commented on Tue Apr 25 2017

New potential focus manager concern:
There are a certain widgets (e.g. dropdown menu, tooltip) that open popups that one would normally expect to close if a user clicks somewhere else on screen (or moves focus away by tabbing).

These could both be handled in something like an onWidgetBlur event, but we would need both dom querying (for document.activeElement) and a document-level focus handler. Should we add that to the list of things a focus manager should do?

---

@mwistrand commented on Tue Apr 25 2017

There's also the limitation that elements cannot receive focus until they are completely visible, which can be problematic when the next item that should receive focus is animated into view (depending on the requirements of the animation).

---

@mwistrand commented on Thu Apr 27 2017

I think I have an idea that might work for focus management and that shouldn't require widgets to interact directly with the DOM. We extend VirtualDomProperties with a callFocus (or just focus) property as well as an onFocusCalled property. callFocus would need to be able to communicate whether its own nodes should be focused, or the first available focusable node should receive focus. onFocusCalled would be used by parents to prevent children from trying to steal focus.

v (but I don't think WidgetBase) would need to be updated to pass its vnode to a scheduleRender method. vnode.properties.afterCreate and vnode.properties.afterUpdate would be wrapped with a function that calls element.focus(), but only if the vnode in question is supposed to receive focus.

A very basic POC is up here (h/t @smhigley for the "get first focusable element" selector).

---

@smhigley commented on Fri May 19 2017

I'd like to propose something like a final list of requirements for the focus manager with their use cases. If there are no more additions, it'd be nice to actually start making this thing:

• Handle calling element.focus()
  Use Case:
  Abstract away touching the DOM from the current implementation in onElementUpdated, and handle the callFocus/onFocusCalled pattern for widget users
• Get list of focusable elements
  Use Case:
  When calling focus on an widget, there may be a case when we need to find the first focusable element. Even if a node is specified, we should probably check that it is focusable before attempting to call .focus(). This approach would also help solve the next few use cases:
• Trap focus within a widget or VNode
  Use Case:
  Dialog, Slidepane, and possibly other modal-ish widgets need to be focus traps. If we have a reference to all focusable nodes, we can listen to blur on the last one instead of needing a document-level focus listener.
• “unfocus” a widget or VNode
  Use Case:
  Any hidden component (e.g. the month popup in Calendar or a collapsed sub-menu) shouldn't be able to receive focus. While this could be achieved by removing the node or setting its css to visibility: hidden, both approaches hinder animation performance. It'd be nice if we could provide a way to loop through all focusable nodes and set tabIndex: -1.
• Widget-level focus and blur events
  Use Case:
  Any action or change that needs to take place when focus enters or leaves the widget as a whole, e.g. closing a tooltip or popup. The implementation should be the same as a focus trap, where we would either need to listen to focus on the document, or focus on the first focusable/blur on the last

---

@rishson commented on Mon May 22 2017

@bitpshr @mwistrand @tomdye @agubler @matt-gadd comments on @smhigley 's proposal above please, so we can progress this.

---

@mwistrand commented on Mon May 22 2017

@smhigley I believe that covers all of my concerns!

---

@smhigley commented on Fri Sep 29 2017

As a quick note, the Accessibility article in dojo.io will need to be updated when this lands: dojo/dojo.io#191 (review)

---

@kitsonk commented on Tue Oct 10 2017

@agubler and @matt-gadd we should resolve this issue if there is any action for the next beta release. If there isn't a concern here, we should close this issue with our reasoning why.

---

@matt-gadd commented on Tue Oct 10 2017

@kitsonk we think this would currently be actioned in dojo/widgets or at least the majority of the requirements listed above. would you like to move it there?

[smhigley]

I still agree with the comment I wrote on May 19, 2017 about what focus-related functionality we need to provide, although I think I would now remove the second point ("Get list of focusable elements").

Here are my current thoughts on implementation details:

I don't believe this all needs to be part of a single "Focus Manager." Re-organizing the points from the comment, I think we could approach it as three independent tools with the following potential approaches to implementation:

1. Handle calling element.focus()

I don't think there's anything wrong with the pattern of our current implementation, but it should be abstracted to a mixin so we no longer touch the DOM in onElementCreated and onElementUpdated in individual widgets. This could also potentially rely on some sort of meta in the background for testing if a node is in the DOM and returning it.

2. Provide meta information about whether focus is within a given node

This can be handled like a combination of Drag and Intersection meta. Similar to Drag, it will need a document-level event listener for the focus event, and like Intersection it can invalidate when focus moves into or out of a provided node.

3. Either trap focus within a node, or remove it from the focus order/AOM

Both these functions rely on inert, which I think we should use and polyfill. We can also either include blockingElements and its polyfill, or create our own implementation. I'm a little dubious about blockingElements actually becoming spec, at least in the foreseeable future. On the other hand, using the polyfill would mean we don't need to traverse the DOM ourselves to apply inert (this is essentially what the polyfill does). I believe this would also make sense as a mixin that adds inert and blocking to the properties interface.

[bitpshr]

I agree with @smhigley's approach listed above. I think a combination of the tools listed would accomplish all focus-specific functionality that Dojo should be responsible for.

I say we start an initial implementation of the pieces involved, though I think most of these will be worked as part of dojo/widget-core. We should open issues as necessary against widget-core and close this discussion issue out.

[bitpshr]

So this isn't lost:

We need to be sure to remove all document-level event handlers (like closing of a Dialog) once focus trapping is correctly implemented.

[smhigley]

Closing this issue, since all points are now addressed in issues on widget-core.

Parts (1) and (2) of my Octobber 24th comment are here: dojo/widget-core#729

Part (3) is here: dojo/widget-core#730