Skip to content

Commit 51f62fd

Browse files
authored
modify isFocusable code to handle natively focusable elements, disabled, and visibility (#11245)
1 parent ee23f72 commit 51f62fd

5 files changed

Lines changed: 100 additions & 21 deletions

File tree

react-common/components/controls/FocusTrap/FocusTrap.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as React from "react";
2-
import { classList, nodeListToArray, findNextFocusableElement, focusLastActive, ContainerProps } from "../../util";
2+
import { classList, nodeListToArray, findNextFocusableElement, focusLastActive, ContainerProps, getFocusableDescendants, getTabbableDescendants } from "../../util";
33
import { addRegion, FocusTrapProvider, removeRegion, useFocusTrapDispatch, useFocusTrapState } from "./context";
44
import { useId } from "../../../hooks/useId";
55

@@ -59,10 +59,7 @@ const FocusTrapInner = (props: FocusTrapProps) => {
5959
}, [])
6060

6161
const getElements = React.useCallback(() => {
62-
let all = nodeListToArray(
63-
includeOutsideTabOrder ? containerRef.current?.querySelectorAll(`[tabindex]`) :
64-
containerRef.current?.querySelectorAll(`[tabindex]:not([tabindex="-1"])`)
65-
);
62+
let all = includeOutsideTabOrder ? getFocusableDescendants(containerRef.current!) : getTabbableDescendants(containerRef.current!);
6663

6764
if (regions.length) {
6865
const regionElements: pxt.Map<Element> = {};

react-common/components/share/ShareInfo.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -501,13 +501,13 @@ export const ShareInfo = (props: ShareInfoProps) => {
501501
heading={lf("Share on WhatsApp")} />
502502
{
503503
pxt.appTarget?.appTheme?.shareToKiosk &&
504-
<Button className="square-button neutral mobile-portrait-hidden"
504+
<Button className="square-button neutral mobile-portrait-hidden social-button"
505505
title={lf("Share to MakeCode Arcade Kiosk")}
506506
leftIcon={"xicon kiosk"}
507507
onClick={handleKioskClick} />
508508
}
509509
{
510-
navigator.share && <Button className="square-button device-share"
510+
navigator.share && <Button className="square-button device-share social-button"
511511
title={lf("Show device share options")}
512512
ariaLabel={lf("Show device share options")}
513513
leftIcon={"icon share"}

react-common/components/share/SocialButton.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ export const SocialButton = (props: SocialButtonProps) => {
9696
/>
9797
);
9898
}
99+
100+
return <></>;
99101
}
100102

101103
const LinkButton = (props: ButtonProps & { heading: string }) => {

react-common/components/util.tsx

Lines changed: 93 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,14 @@ export function screenToSVGCoord(ref: SVGSVGElement, coord: ClientCoordinates) {
9393
return screenCoord.matrixTransform(ref.getScreenCTM().inverse());
9494
}
9595

96-
export function findNextFocusableElement(elements: HTMLElement[], focusedIndex: number, index: number, forward: boolean, isFocusable?: (e: HTMLElement) => boolean): HTMLElement {
96+
export function findNextFocusableElement(elements: HTMLElement[], focusedIndex: number, index: number, forward: boolean, filter?: (e: HTMLElement) => boolean): HTMLElement {
9797
const increment = forward ? 1 : -1;
9898
const element = elements[index];
9999
// in this case, there are no focusable elements
100100
if (focusedIndex === index) {
101101
return element;
102102
}
103-
if (isFocusable ? isFocusable(element) : isVisible(element)) {
103+
if (filter ? filter(element) : isVisible(element)) {
104104
return element;
105105
} else {
106106
if (index + increment >= elements.length) {
@@ -111,25 +111,107 @@ export function findNextFocusableElement(elements: HTMLElement[], focusedIndex:
111111
index += increment;
112112
}
113113
}
114-
return findNextFocusableElement(elements, focusedIndex, index, forward, isFocusable);
114+
return findNextFocusableElement(elements, focusedIndex, index, forward, filter);
115115
}
116116

117-
function isVisible(e: HTMLElement): boolean {
117+
export function getFocusableDescendants(container: Element): Element[] {
118+
return getVisibleDescendants(container, isFocusableIfVisible);
119+
}
120+
121+
export function getTabbableDescendants(container: Element): Element[] {
122+
return getVisibleDescendants(container, isTabbableIfVisible);
123+
}
124+
125+
export function getVisibleDescendants(container: Element, filter: (e: Element) => boolean): Element[] {
126+
if (!isVisible(container)) {
127+
return [];
128+
}
129+
130+
const walker = document.createTreeWalker(
131+
container,
132+
NodeFilter.SHOW_ELEMENT,
133+
node => {
134+
// If not visible, don't bother walking the subtree
135+
if (!isVisible(node as Element, false)) {
136+
return NodeFilter.FILTER_REJECT;
137+
}
138+
return filter(node as Element) ? NodeFilter.FILTER_ACCEPT : NodeFilter.FILTER_SKIP;
139+
}
140+
);
141+
142+
const elements: Element[] = [];
143+
let currentNode: Node | null = walker.nextNode();
144+
while (currentNode) {
145+
elements.push(currentNode as Element);
146+
currentNode = walker.nextNode();
147+
}
148+
return elements;
149+
}
150+
151+
function isVisible(e: Element, checkParent = true): boolean {
118152
if ((e as any).checkVisibility) {
119153
return (e as any).checkVisibility({ visibilityProperty: true });
120154
}
121155
const style = getComputedStyle(e);
122-
return style.display !== "none" && style.visibility !== "hidden";
156+
if (style.display === "none" || style.visibility === "hidden") {
157+
return false;
158+
}
159+
160+
if (checkParent && e.parentElement) {
161+
return isVisible(e.parentElement, checkParent);
162+
}
163+
return true;
123164
}
124165

125-
export function isFocusable(e: HTMLElement) {
166+
export function isFocusable(e: Element) {
167+
return isFocusableIfVisible(e) && isVisible(e);
168+
}
169+
170+
function isFocusableIfVisible(e: Element) {
171+
if (isDisabled(e)) return false;
172+
173+
// There are some edge cases here like <summary> elements and
174+
// span elements with the `user-modify` attribute but we don't use
175+
// those anyway. This should cover the vast majority
176+
if (
177+
e.hasAttribute("tabindex") ||
178+
(e.tagName === "A" && (e.hasAttribute("href") || e.hasAttributeNS("xlink", "href"))) ||
179+
e.tagName === "BUTTON" ||
180+
e.tagName === "INPUT" ||
181+
e.tagName === "SELECT" ||
182+
e.tagName === "TEXTAREA" ||
183+
e.tagName === "IFRAME" ||
184+
e.tagName === "EMBED" ||
185+
e.tagName === "OBJECT" ||
186+
(e.tagName === "DIV" && e.hasAttribute("contenteditable") && e.getAttribute("contenteditable") !== "false") ||
187+
((e.tagName === "AUDIO" || e.tagName === "VIDEO") && e.hasAttribute("controls"))
188+
) {
189+
return true;
190+
}
191+
192+
return false;
193+
}
194+
195+
196+
export function isDisabled(e: Element) {
126197
if (e) {
127-
return (e.getAttribute("data-isfocusable") === "true"
128-
|| e.tabIndex !== -1)
129-
&& getComputedStyle(e).display !== "none";
130-
} else {
131-
return false;
198+
if (e.hasAttribute("disabled")) return true;
199+
}
200+
return false;
201+
}
202+
203+
export function isTabbable(e: Element) {
204+
return isTabbableIfVisible(e) && isVisible(e);
205+
}
206+
207+
function isTabbableIfVisible(e: Element) {
208+
if (isFocusableIfVisible(e)) {
209+
if (e.hasAttribute("tabindex")) {
210+
return parseInt(e.getAttribute("tabindex")!) >= 0;
211+
}
212+
return true;
132213
}
214+
return false;
133215
}
134216

135217
export function focusLastActive(el: HTMLElement) {

react-common/styles/controls/Button.less

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,7 @@
9595
.common-button.orange:focus-visible::after,
9696
.common-button.red:focus-visible::after,
9797
.common-button.green:focus-visible::after,
98-
.common-button.facebook:focus-visible::after,
99-
.common-button.twitter:focus-visible::after,
100-
.common-button.discourse:focus-visible::after {
98+
.common-button.social-button:focus-visible::after {
10199
outline: @buttonFocusOutlineDarkBackground;
102100
}
103101

0 commit comments

Comments
 (0)