From b39860b03f7ac563293f34f1008826365cac8d29 Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Tue, 12 Nov 2024 14:18:19 -0500 Subject: [PATCH 1/5] fix(Tag): Add keyboard accessibility --- .../accessibility/useInteractiveAttributes.ts | 87 +++++++++++++++++++ .../core/src/components/button/buttons.tsx | 77 ++-------------- packages/core/src/components/tag/tag.tsx | 8 +- 3 files changed, 99 insertions(+), 73 deletions(-) create mode 100644 packages/core/src/accessibility/useInteractiveAttributes.ts diff --git a/packages/core/src/accessibility/useInteractiveAttributes.ts b/packages/core/src/accessibility/useInteractiveAttributes.ts new file mode 100644 index 0000000000..3960e6c192 --- /dev/null +++ b/packages/core/src/accessibility/useInteractiveAttributes.ts @@ -0,0 +1,87 @@ +/* ! + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + */ + +import * as React from "react"; + +import { mergeRefs, Utils } from "../common"; + +type InteractiveHTMLAttributes = Pick< + React.HTMLAttributes, + "onBlur" | "onClick" | "onFocus" | "onKeyDown" | "onKeyUp" | "tabIndex" +>; + +interface InteractiveComponentProps extends InteractiveHTMLAttributes { + active?: boolean | undefined; +} + +interface InteractiveAttributes extends InteractiveHTMLAttributes { + ref: React.Ref; +} + +export function useInteractiveAttributes( + interactive: boolean, + props: InteractiveComponentProps, + ref: React.Ref, +): [active: boolean, interactiveProps: InteractiveAttributes] { + const { active, onClick, onFocus, onKeyDown, onKeyUp, onBlur, tabIndex = 0 } = props; + // the current key being pressed + const [currentKeyPressed, setCurrentKeyPressed] = React.useState(); + // whether the button is in "active" state + const [isActive, setIsActive] = React.useState(false); + // our local ref for the interactive element, merged with the consumer's own ref (if supplied) in this hook's return value + const elementRef = React.useRef(null); + + const handleBlur = React.useCallback( + (e: React.FocusEvent) => { + if (isActive) { + setIsActive(false); + } + + onBlur?.(e); + }, + [isActive, onBlur], + ); + + const handleKeyDown = React.useCallback( + (e: React.KeyboardEvent) => { + if (Utils.isKeyboardClick(e)) { + e.preventDefault(); + if (e.key !== currentKeyPressed) { + setIsActive(true); + } + } + + setCurrentKeyPressed(e.key); + onKeyDown?.(e); + }, + [currentKeyPressed, onKeyDown], + ); + + const handleKeyUp = React.useCallback( + (e: React.KeyboardEvent) => { + if (Utils.isKeyboardClick(e)) { + setIsActive(false); + elementRef.current?.click(); + } + setCurrentKeyPressed(undefined); + onKeyUp?.(e); + }, + [onKeyUp, elementRef], + ); + + const resolvedActive = interactive && (active || isActive); + + return [ + resolvedActive, + { + onBlur: handleBlur, + onClick: interactive ? onClick : undefined, + onFocus: interactive ? onFocus : undefined, + onKeyDown: handleKeyDown, + onKeyUp: handleKeyUp, + ref: mergeRefs(elementRef, ref), + tabIndex: interactive ? tabIndex : -1, + }, + ]; +} diff --git a/packages/core/src/components/button/buttons.tsx b/packages/core/src/components/button/buttons.tsx index 4ddeae7c5f..b1b56ae203 100644 --- a/packages/core/src/components/button/buttons.tsx +++ b/packages/core/src/components/button/buttons.tsx @@ -17,9 +17,9 @@ import classNames from "classnames"; import * as React from "react"; +import { useInteractiveAttributes } from "../../accessibility/useInteractiveAttributes"; import { Classes, Utils } from "../../common"; import { DISPLAYNAME_PREFIX, removeNonHTMLProps } from "../../common/props"; -import { mergeRefs } from "../../common/refs"; import { Icon } from "../icon/icon"; import { Spinner, SpinnerSize } from "../spinner/spinner"; import { Text } from "../text/text"; @@ -49,7 +49,7 @@ Button.displayName = `${DISPLAYNAME_PREFIX}.Button`; */ export const AnchorButton: React.FC = React.forwardRef( (props, ref) => { - const { href, tabIndex = 0 } = props; + const { href } = props; const commonProps = useSharedButtonAttributes(props, ref); return ( @@ -59,7 +59,6 @@ export const AnchorButton: React.FC = React.forwardRef {renderButtonContents(props)} @@ -73,69 +72,17 @@ AnchorButton.displayName = `${DISPLAYNAME_PREFIX}.AnchorButton`; */ function useSharedButtonAttributes( props: E extends HTMLAnchorElement ? AnchorButtonProps : ButtonProps, - ref: React.Ref, + passedRef: React.Ref, ) { - const { - active = false, - alignText, - fill, - large, - loading = false, - minimal, - onBlur, - onKeyDown, - onKeyUp, - outlined, - small, - tabIndex, - } = props; + const { alignText, fill, large, loading = false, minimal, outlined, small } = props; const disabled = props.disabled || loading; - // the current key being pressed - const [currentKeyPressed, setCurrentKeyPressed] = React.useState(); - // whether the button is in "active" state - const [isActive, setIsActive] = React.useState(false); - // our local ref for the button element, merged with the consumer's own ref (if supplied) in this hook's return value - const buttonRef = React.useRef(null); - - const handleBlur = React.useCallback( - (e: React.FocusEvent) => { - if (isActive) { - setIsActive(false); - } - onBlur?.(e); - }, - [isActive, onBlur], - ); - const handleKeyDown = React.useCallback( - (e: React.KeyboardEvent) => { - if (Utils.isKeyboardClick(e)) { - e.preventDefault(); - if (e.key !== currentKeyPressed) { - setIsActive(true); - } - } - setCurrentKeyPressed(e.key); - onKeyDown?.(e); - }, - [currentKeyPressed, onKeyDown], - ); - const handleKeyUp = React.useCallback( - (e: React.KeyboardEvent) => { - if (Utils.isKeyboardClick(e)) { - setIsActive(false); - buttonRef.current?.click(); - } - setCurrentKeyPressed(undefined); - onKeyUp?.(e); - }, - [onKeyUp], - ); + const [active, interactiveProps] = useInteractiveAttributes(!disabled, props, passedRef); const className = classNames( Classes.BUTTON, { - [Classes.ACTIVE]: !disabled && (active || isActive), + [Classes.ACTIVE]: active, [Classes.DISABLED]: disabled, [Classes.FILL]: fill, [Classes.LARGE]: large, @@ -150,15 +97,9 @@ function useSharedButtonAttributes( {text} {children} - // - // {text} - // {children} - // )} diff --git a/packages/core/src/components/tag/tag.tsx b/packages/core/src/components/tag/tag.tsx index 9d0cac42d7..d7e66607b9 100644 --- a/packages/core/src/components/tag/tag.tsx +++ b/packages/core/src/components/tag/tag.tsx @@ -19,6 +19,7 @@ import * as React from "react"; import type { IconName } from "@blueprintjs/icons"; +import { useInteractiveAttributes } from "../../accessibility/useInteractiveAttributes"; import { Classes, DISPLAYNAME_PREFIX, type IntentProps, type MaybeElement, type Props, Utils } from "../../common"; import { isReactNodeEmpty } from "../../common/utils"; import { Icon } from "../icon/icon"; @@ -72,13 +73,12 @@ export interface TagProps */ export const Tag: React.FC = React.forwardRef((props, ref) => { const { - active, children, className, fill, icon, intent, - interactive, + interactive = false, large, minimal, multiline, @@ -92,6 +92,8 @@ export const Tag: React.FC = React.forwardRef((props, ref) => { const isRemovable = Utils.isFunction(onRemove); + const [active, interactiveProps] = useInteractiveAttributes(interactive, props, ref); + const tagClasses = classNames( Classes.TAG, Classes.intentClass(intent), @@ -107,7 +109,7 @@ export const Tag: React.FC = React.forwardRef((props, ref) => { ); return ( - + {!isReactNodeEmpty(children) && ( From 5764e90416eca88f25fe857a747ffde9f0d1ee7e Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Tue, 12 Nov 2024 17:42:20 -0500 Subject: [PATCH 2/5] Fix tests --- packages/core/src/accessibility/useInteractiveAttributes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/accessibility/useInteractiveAttributes.ts b/packages/core/src/accessibility/useInteractiveAttributes.ts index 3960e6c192..20213ce6c3 100644 --- a/packages/core/src/accessibility/useInteractiveAttributes.ts +++ b/packages/core/src/accessibility/useInteractiveAttributes.ts @@ -24,7 +24,7 @@ export function useInteractiveAttributes( props: InteractiveComponentProps, ref: React.Ref, ): [active: boolean, interactiveProps: InteractiveAttributes] { - const { active, onClick, onFocus, onKeyDown, onKeyUp, onBlur, tabIndex = 0 } = props; + const { active, onClick, onFocus, onKeyDown, onKeyUp, onBlur, tabIndex } = props; // the current key being pressed const [currentKeyPressed, setCurrentKeyPressed] = React.useState(); // whether the button is in "active" state From 9a030516ed3257664c30a4463bbfda788a9d1245 Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Tue, 12 Nov 2024 17:47:26 -0500 Subject: [PATCH 3/5] Fix --- packages/core/src/accessibility/useInteractiveAttributes.ts | 3 ++- packages/core/src/components/tag/tag.tsx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/src/accessibility/useInteractiveAttributes.ts b/packages/core/src/accessibility/useInteractiveAttributes.ts index 20213ce6c3..c0a3bf46d2 100644 --- a/packages/core/src/accessibility/useInteractiveAttributes.ts +++ b/packages/core/src/accessibility/useInteractiveAttributes.ts @@ -23,8 +23,9 @@ export function useInteractiveAttributes( interactive: boolean, props: InteractiveComponentProps, ref: React.Ref, + defaultTabIndex?: number, ): [active: boolean, interactiveProps: InteractiveAttributes] { - const { active, onClick, onFocus, onKeyDown, onKeyUp, onBlur, tabIndex } = props; + const { active, onClick, onFocus, onKeyDown, onKeyUp, onBlur, tabIndex = defaultTabIndex } = props; // the current key being pressed const [currentKeyPressed, setCurrentKeyPressed] = React.useState(); // whether the button is in "active" state diff --git a/packages/core/src/components/tag/tag.tsx b/packages/core/src/components/tag/tag.tsx index d7e66607b9..144cb7adf0 100644 --- a/packages/core/src/components/tag/tag.tsx +++ b/packages/core/src/components/tag/tag.tsx @@ -92,7 +92,7 @@ export const Tag: React.FC = React.forwardRef((props, ref) => { const isRemovable = Utils.isFunction(onRemove); - const [active, interactiveProps] = useInteractiveAttributes(interactive, props, ref); + const [active, interactiveProps] = useInteractiveAttributes(interactive, props, ref, 0); const tagClasses = classNames( Classes.TAG, From eec0ece65dc2b1061ae0639166fa846b86525dcb Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Tue, 12 Nov 2024 22:52:23 +0000 Subject: [PATCH 4/5] Add generated changelog entries --- packages/core/changelog/@unreleased/pr-7060.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/core/changelog/@unreleased/pr-7060.v2.yml diff --git a/packages/core/changelog/@unreleased/pr-7060.v2.yml b/packages/core/changelog/@unreleased/pr-7060.v2.yml new file mode 100644 index 0000000000..d12e967caa --- /dev/null +++ b/packages/core/changelog/@unreleased/pr-7060.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: 'fix(Tag): Add keyboard accessibility' + links: + - https://github.com/palantir/blueprint/pull/7060 From 63b9c1a40715ca4673304371a72ee73732a85928 Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Wed, 13 Nov 2024 14:35:16 -0500 Subject: [PATCH 5/5] CR --- packages/core/src/accessibility/useInteractiveAttributes.ts | 2 +- packages/core/src/components/button/buttons.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/accessibility/useInteractiveAttributes.ts b/packages/core/src/accessibility/useInteractiveAttributes.ts index c0a3bf46d2..d2022e51b2 100644 --- a/packages/core/src/accessibility/useInteractiveAttributes.ts +++ b/packages/core/src/accessibility/useInteractiveAttributes.ts @@ -30,7 +30,7 @@ export function useInteractiveAttributes( const [currentKeyPressed, setCurrentKeyPressed] = React.useState(); // whether the button is in "active" state const [isActive, setIsActive] = React.useState(false); - // our local ref for the interactive element, merged with the consumer's own ref (if supplied) in this hook's return value + // our local ref for the interactive element, merged with the consumer's own ref in this hook's return value const elementRef = React.useRef(null); const handleBlur = React.useCallback( diff --git a/packages/core/src/components/button/buttons.tsx b/packages/core/src/components/button/buttons.tsx index b1b56ae203..226b97d57c 100644 --- a/packages/core/src/components/button/buttons.tsx +++ b/packages/core/src/components/button/buttons.tsx @@ -72,12 +72,12 @@ AnchorButton.displayName = `${DISPLAYNAME_PREFIX}.AnchorButton`; */ function useSharedButtonAttributes( props: E extends HTMLAnchorElement ? AnchorButtonProps : ButtonProps, - passedRef: React.Ref, + ref: React.Ref, ) { const { alignText, fill, large, loading = false, minimal, outlined, small } = props; const disabled = props.disabled || loading; - const [active, interactiveProps] = useInteractiveAttributes(!disabled, props, passedRef); + const [active, interactiveProps] = useInteractiveAttributes(!disabled, props, ref); const className = classNames( Classes.BUTTON,