Skip to content

Commit

Permalink
Remove attempts to determine is-focusable
Browse files Browse the repository at this point in the history
Instead, we'll just call .focus() and let the browser decide if it's focusable.
  • Loading branch information
Brian Vaughn committed Apr 18, 2020
1 parent c10b65a commit f116626
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -937,9 +937,9 @@ No matching component was found for:
expect(didFocus).toBe(false);
});

it('should return false if the only focusable elements are visible', () => {
it('should return false if the only focusable elements are hidden', () => {
function Example() {
return <button style={{display: 'none'}}>not clickable</button>;
return <button hidden={true}>not clickable</button>;
}

render(<Example />, container);
Expand Down Expand Up @@ -968,7 +968,7 @@ No matching component was found for:
}
function FirstChild() {
return (
<button style={{display: 'none'}} onFocus={handleFirstFocus}>
<button hidden={true} onFocus={handleFirstFocus}>
not clickable
</button>
);
Expand Down
43 changes: 6 additions & 37 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -1312,49 +1312,18 @@ export function isHiddenSubtree(fiber: Fiber): boolean {
}

export function setFocusIfFocusable(node: Instance): boolean {
if (node.nodeType !== Node.ELEMENT_NODE) {
// Text and comment nodes aren't focusable.
// Technically this check should not be necessary,
// since React treats elements (Instances) and text (TextInstance) differently.
return false;
}

if (((node: any): HTMLInputElement).disabled === true) {
// Disabled inputs can't be focused.
return false;
}

const element = ((node: any): HTMLElement);

if (element.tabIndex === null || element.tabIndex < 0) {
// The HTML spec says that negative tab index values indicate an element should be,
// "click focusable but not sequentially focusable".
// https://html.spec.whatwg.org/multipage/interaction.html#the-tabindex-attribute
//
// The HTML focusable spec also says,
// "User agents should consider focusable areas with non-null tabindex values to be click focusable."
// https://html.spec.whatwg.org/multipage/interaction.html#focusable
//
// Despite this, it seems like some browsers (e.g. Chrome, Firefox) return -1 even for elements
// that don't accept focus, like HTMLImageElement or the outermost HTMLElement tag.
// I think this method should (at least for now) only concern itself with "sequentially focusable" elements.
// https://html.spec.whatwg.org/multipage/interaction.html#sequentially-focusable
return false;
}

if (element.offsetWidth === 0 || element.offsetHeight === 0) {
// Hidden items can't be focused.
return false;
}

// At this point we assume the element accepts focus, so let's try and see.
// Listen for a "focus" event to verify that focus was set.
// The logic for determining if an element is focusable is kind of complex,
// and since we want to actually change focus anyway- we can just skip it.
// Instead we'll just listen for a "focus" event to verify that focus was set.
//
// We could compare the node to document.activeElement after focus,
// but this would not handle the case where application code managed focus to automatically blur.
let didFocus = false;
const handleFocus = () => {
didFocus = true;
};

const element = ((node: any): HTMLElement);
try {
element.addEventListener('focus', handleFocus);
element.focus();
Expand Down

0 comments on commit f116626

Please sign in to comment.