RAC Pending Button #6435

snowystinger · 2024-05-23T03:32:22Z

I've opted to re-implement it in RAC as opposed to pushing it down into the hooks because the hacks we use to work around AT require a bunch of dom nodes, which either leads to a lot of oddly named collections of element props being returned by the useButton hook, or a new hook which handles it. Either way results in API additions which would be not great to maintain.

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

rspbot · 2024-05-23T03:44:55Z

Build successful! 🎉

packages/react-aria-components/src/Button.tsx

# Conflicts: # packages/react-aria-components/stories/Button.stories.tsx

rspbot · 2024-06-11T01:36:15Z

Build successful! 🎉

rspbot · 2024-06-11T04:45:37Z

Build successful! 🎉

rspbot · 2024-06-11T04:56:14Z

Build successful! 🎉

packages/react-aria-components/package.json

packages/react-aria-components/src/Button.tsx

# Conflicts: # packages/react-aria-components/package.json

rspbot · 2024-06-23T22:58:18Z

Build successful! 🎉

rspbot · 2024-06-23T22:58:39Z

Build successful! 🎉

packages/react-aria-components/src/Button.tsx

rspbot · 2024-06-30T00:18:03Z

Build successful! 🎉

rspbot · 2024-07-25T23:49:11Z

Build successful! 🎉

rspbot · 2024-08-20T06:33:48Z

Build successful! 🎉

snowystinger · 2024-08-20T06:33:52Z

packages/@react-aria/live-announcer/src/LiveAnnouncer.tsx

 /**
 * Announces the message using screen reader technology.
 */
 export function announce(
  message: string,
  assertiveness: Assertiveness = 'assertive',
-  timeout = LIVEREGION_TIMEOUT_DELAY
+  timeout = LIVEREGION_TIMEOUT_DELAY,
+  isIds = false // better name? better description?


added so i could pass ids to live announcer instead of a string message. this allows me to announce anything the same way without needed to extract the textContent or anything else

announceNode/Element or something like that? Maybe this could be a mode arg or something that takes 'string | id' and defaults to string?

snowystinger · 2024-08-20T06:34:40Z

packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js

@@ -3228,7 +3228,9 @@ describe('SearchAutocomplete', function () {

        let listbox = getByRole('listbox');
        expect(listbox).toBeVisible();
-        expect(screen.getAllByRole('log')).toHaveLength(2);
+        expect(announce).toHaveBeenCalledTimes(2);


these tests were honestly checking something silly, so I've updated them to check useful information

snowystinger · 2024-08-20T06:35:02Z

packages/@react-spectrum/color/test/ColorPicker.test.js

@@ -40,7 +40,7 @@ describe('ColorPicker', function () {

    let button = getByRole('button');
    expect(button).toHaveTextContent('Fill');
-    expect(within(button).getByRole('img')).toHaveAttribute('aria-label', 'vibrant red');
+    expect(within(button).getByLabelText('vibrant red')).toBeInTheDocument();


was a brittle test, these will last better

rspbot · 2024-08-20T06:45:25Z

Build successful! 🎉

rspbot · 2024-08-20T06:48:14Z

Build successful! 🎉

LFDanLu

Just some small comments, but otherwise verified the behavior on Chrome and Safari

LFDanLu · 2024-08-28T23:35:50Z

packages/@react-aria/live-announcer/src/LiveAnnouncer.tsx

 /**
 * Announces the message using screen reader technology.
 */
 export function announce(
  message: string,
  assertiveness: Assertiveness = 'assertive',
-  timeout = LIVEREGION_TIMEOUT_DELAY
+  timeout = LIVEREGION_TIMEOUT_DELAY,
+  isIds = false // better name? better description?


announceNode/Element or something like that? Maybe this could be a mode arg or something that takes 'string | id' and defaults to string?

packages/react-aria-components/stories/Button.stories.tsx

packages/react-aria-components/src/Button.tsx

rspbot · 2024-08-29T04:51:47Z

Build successful! 🎉

# Conflicts: # packages/react-aria-components/package.json # yarn.lock

…into rac-pending-button

rspbot · 2024-08-29T20:19:55Z

Build successful! 🎉

LFDanLu

LGTM, open to feedback on the new LiveAnnouncer option naming

rspbot · 2024-08-30T00:30:49Z

Build successful! 🎉

rspbot · 2024-08-30T00:30:50Z

## API Changes

react-aria-components

/react-aria-components:Button

 Button {
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode | ((ButtonRenderProps & {
     defaultChildren: ReactNode | undefined
 })) => ReactNode
   className?: string | ((ButtonRenderProps & {
     defaultClassName: string | undefined
 })) => string
   excludeFromTabOrder?: boolean
   form?: string
   formAction?: string
   formEncType?: string
   formMethod?: string
   formNoValidate?: boolean
   formTarget?: string
   id?: string
   isDisabled?: boolean
+  isPending?: boolean
   name?: string
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onHoverEnd?: (HoverEvent) => void
   onHoverStart?: (HoverEvent) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   slot?: string | null
   style?: CSSProperties | ((ButtonRenderProps & {
     defaultStyle: CSSProperties
 })) => CSSProperties
   type?: 'button' | 'submit' | 'reset' = 'button'
   value?: string
 }

/react-aria-components:ButtonProps

 ButtonProps {
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode | ((ButtonRenderProps & {
     defaultChildren: ReactNode | undefined
 })) => ReactNode
   className?: string | ((ButtonRenderProps & {
     defaultClassName: string | undefined
 })) => string
   excludeFromTabOrder?: boolean
   form?: string
   formAction?: string
   formEncType?: string
   formMethod?: string
   formNoValidate?: boolean
   formTarget?: string
   id?: string
   isDisabled?: boolean
+  isPending?: boolean
   name?: string
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onHoverEnd?: (HoverEvent) => void
   onHoverStart?: (HoverEvent) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   slot?: string | null
   style?: CSSProperties | ((ButtonRenderProps & {
     defaultStyle: CSSProperties
 })) => CSSProperties
   type?: 'button' | 'submit' | 'reset' = 'button'
   value?: string
 }

/react-aria-components:ButtonRenderProps

 ButtonRenderProps {
   isDisabled: boolean
   isFocusVisible: boolean
   isFocused: boolean
   isHovered: boolean
+  isPending?: boolean
   isPressed: boolean
 }

@react-aria/live-announcer

/@react-aria/live-announcer:announce

 announce {
   message: string
   assertiveness: Assertiveness
   timeout: any
+  mode: 'message' | 'ids'
   returnVal: undefined
 }

@react-spectrum/s2

/@react-spectrum/s2:ActionButton

 ActionButton {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode
   excludeFromTabOrder?: boolean
   form?: string
   formAction?: string
   formEncType?: string
   formMethod?: string
   formNoValidate?: boolean
   formTarget?: string
   id?: string
   isDisabled?: boolean
+  isPending?: boolean
   isQuiet?: boolean
   name?: string
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   staticColor?: 'black' | 'white'
   styles?: StylesProp
   type?: 'button' | 'submit' | 'reset' = 'button'
   value?: string
 }

/@react-spectrum/s2:Button

 Button {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode
   excludeFromTabOrder?: boolean
   fillStyle?: 'fill' | 'outline' = 'fill'
   form?: string
   formAction?: string
   formEncType?: string
   formMethod?: string
   formNoValidate?: boolean
   formTarget?: string
   id?: string
   isDisabled?: boolean
+  isPending?: boolean
   name?: string
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   staticColor?: 'white' | 'black'
   styles?: StylesProp
   type?: 'button' | 'submit' | 'reset' = 'button'
   value?: string
   variant?: 'primary' | 'secondary' | 'accent' | 'negative' = 'primary'
 }

/@react-spectrum/s2:ActionButtonProps

 ActionButtonProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode
   excludeFromTabOrder?: boolean
   form?: string
   formAction?: string
   formEncType?: string
   formMethod?: string
   formNoValidate?: boolean
   formTarget?: string
   id?: string
   isDisabled?: boolean
+  isPending?: boolean
   isQuiet?: boolean
   name?: string
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   staticColor?: 'black' | 'white'
   styles?: StylesProp
   type?: 'button' | 'submit' | 'reset' = 'button'
   value?: string
 }

/@react-spectrum/s2:ButtonProps

 ButtonProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode
   excludeFromTabOrder?: boolean
   fillStyle?: 'fill' | 'outline' = 'fill'
   form?: string
   formAction?: string
   formEncType?: string
   formMethod?: string
   formNoValidate?: boolean
   formTarget?: string
   id?: string
   isDisabled?: boolean
+  isPending?: boolean
   name?: string
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   staticColor?: 'white' | 'black'
   styles?: StylesProp
   type?: 'button' | 'submit' | 'reset' = 'button'
   value?: string
   variant?: 'primary' | 'secondary' | 'accent' | 'negative' = 'primary'
 }

devongovett · 2024-08-30T12:08:31Z

packages/@react-aria/live-announcer/src/LiveAnnouncer.tsx

+
+// singleton, setup immediately so that the DOM is primed for the first announcement as soon as possible
+// Safari has a race condition where the first announcement is not read if we wait until the first announce call
+let liveAnnouncer: LiveAnnouncer | null = new LiveAnnouncer();


That means it can never be tree shaken anymore

ah bugger, ok, will look into that as well

devongovett · 2024-08-30T12:08:52Z

packages/@react-aria/live-announcer/src/LiveAnnouncer.tsx

 /**
 * Announces the message using screen reader technology.
 */
 export function announce(
  message: string,
  assertiveness: Assertiveness = 'assertive',
-  timeout = LIVEREGION_TIMEOUT_DELAY
+  timeout = LIVEREGION_TIMEOUT_DELAY,
+  mode: 'message' | 'ids' = 'message'


be able to send in a set of id's for use in aria-labelledby which is then live announced
vs sending the message in directly. this allows us to send messages which would otherwise require textContent or some other not friendly way of converting HTML to a string

devongovett · 2024-08-30T12:13:20Z

packages/react-aria-components/docs/Button.mdx

+    return () => {
+      clearTimeout(timeout.current);
+    };
+  }, []);


Since it's just a demo we can probably assume that it's not gonna unmount before the timeout. Nice to keep the example simple.

devongovett · 2024-08-30T12:25:08Z

packages/react-aria-components/src/Button.tsx

+      <Provider
+        values={[
+          [TextContext, {id: contentId, ref: textCallbackRef}],
+          [ProgressBarContext, {id: progressId, style: {display: isPending ? undefined : 'none'}, isIndeterminate: true, ref: progressCallbackRef}]


What if the developer unmounts the progress bar rather than using display none? Isn't it a bit of an assumption that they'd do it this way?

that's what the useEffect thrown errors are for, to tell people it won't work that way
I agree though, this is less than ideal

devongovett · 2024-08-30T12:25:29Z

packages/react-aria-components/src/Button.tsx

+        ]}>
+        {renderProps.children}
+      </Provider>
+      <VisuallyHidden>


Would be nice to avoid extra elements in all buttons when they aren't using pending state

yep, now that i'm using the LiveAnnouncer, I'm trying to see if I can just not render this unless we're actually pending

devongovett · 2024-08-30T12:27:03Z

packages/react-aria-components/src/Button.tsx

+
+  const isPendingAriaLiveLabel = `${hasAriaLabel ? buttonProps['aria-label'] : ''}`.trim();
+  let isPendingAriaLiveLabelledby = hasAriaLabel ? (buttonProps['aria-labelledby']?.replace(buttonId, safariDupeLabellingId) ?? safariDupeLabellingId) : `${contentId} ${safariDupeLabellingId}`.trim();
+  isPendingAriaLiveLabelledby = isPendingAriaLiveLabelledby + ' ' + progressId;


Would be nice to document what all this stuff does

snowystinger added 4 commits May 23, 2024 11:32

RAC Pending Button

b437282

fix types

844ff79

Attach button id

66b1a7d

make prop name match others

c9f6fb6

devongovett reviewed May 29, 2024

View reviewed changes

packages/react-aria-components/src/Button.tsx Outdated Show resolved Hide resolved

packages/react-aria-components/src/Button.tsx Outdated Show resolved Hide resolved

packages/react-aria-components/src/Button.tsx Outdated Show resolved Hide resolved

snowystinger added 3 commits June 5, 2024 17:54

Use function for render pending state

2836fc1

fix story example

85e1efc

Merge branch 'main' into rac-pending-button

48e9efe

# Conflicts: # packages/react-aria-components/stories/Button.stories.tsx

snowystinger added 2 commits June 11, 2024 14:28

Remove need for timer

116e006

Merge branch 'main' into rac-pending-button

9c2af30

LFDanLu reviewed Jun 21, 2024

View reviewed changes

packages/react-aria-components/package.json Outdated Show resolved Hide resolved

packages/react-aria-components/src/Button.tsx Outdated Show resolved Hide resolved

snowystinger added 2 commits June 24, 2024 08:36

Merge branch 'main' into rac-pending-button

a6da930

# Conflicts: # packages/react-aria-components/package.json

remove erroneous dependencies

c74587f

levrik reviewed Jun 24, 2024

View reviewed changes

packages/react-aria-components/src/Button.tsx Outdated Show resolved Hide resolved

Merge branch 'main' into rac-pending-button

c8a0e37

akyllus approved these changes Jul 1, 2024

View reviewed changes

devongovett force-pushed the main branch 2 times, most recently from 8cb1a5d to 3013156 Compare July 23, 2024 22:43

Merge branch 'main' into rac-pending-button

5250fc7

snowystinger added 2 commits August 9, 2024 22:29

Move to required children and context

c6ff5cf

fix tests and lint

6f7db28

snowystinger added 5 commits August 20, 2024 15:33

use a live announcer, add docs

d86266e

fix lockfile

17cffa6

fix tests

0e953f4

fix lint

2786f4f

revert unneeded translations

9a85204

snowystinger commented Aug 20, 2024

View reviewed changes

snowystinger mentioned this pull request Aug 21, 2024

Allow disabled items to be focused #3662

Closed

LFDanLu reviewed Aug 28, 2024

View reviewed changes

review comments

bbbba67

snowystinger added 5 commits August 29, 2024 15:46

Merge branch 'main' into rac-pending-button

a2c1daf

# Conflicts: # packages/react-aria-components/package.json # yarn.lock

fix strictmode

7b06d83

Merge branch 'main' into rac-pending-button

79d2ac4

fix lint

5204aca

Merge branch 'rac-pending-button' of github.com:adobe/react-spectrum …

d727496

…into rac-pending-button

LFDanLu approved these changes Aug 29, 2024

View reviewed changes

ktabors approved these changes Aug 29, 2024

View reviewed changes

Merge branch 'main' into rac-pending-button

2278646

snowystinger merged commit 3555336 into main Aug 30, 2024
29 checks passed

snowystinger deleted the rac-pending-button branch August 30, 2024 01:02

devongovett reviewed Aug 30, 2024

View reviewed changes

snowystinger mentioned this pull request Sep 3, 2024

Feature - RAC and S2 Pending Button #6987

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RAC Pending Button #6435

RAC Pending Button #6435

snowystinger commented May 23, 2024 •

edited

Loading

rspbot commented May 23, 2024

rspbot commented Jun 11, 2024

rspbot commented Jun 11, 2024

rspbot commented Jun 11, 2024

rspbot commented Jun 23, 2024

rspbot commented Jun 23, 2024

rspbot commented Jun 30, 2024

rspbot commented Jul 25, 2024

rspbot commented Aug 20, 2024

snowystinger Aug 20, 2024

LFDanLu Aug 28, 2024

snowystinger Aug 20, 2024

snowystinger Aug 20, 2024

rspbot commented Aug 20, 2024

rspbot commented Aug 20, 2024

LFDanLu left a comment

LFDanLu Aug 28, 2024

rspbot commented Aug 29, 2024

rspbot commented Aug 29, 2024

LFDanLu left a comment

rspbot commented Aug 30, 2024

rspbot commented Aug 30, 2024

devongovett Aug 30, 2024

snowystinger Aug 31, 2024

devongovett Aug 30, 2024

snowystinger Aug 31, 2024

devongovett Aug 30, 2024

devongovett Aug 30, 2024

snowystinger Aug 31, 2024

devongovett Aug 30, 2024

snowystinger Aug 31, 2024

devongovett Aug 30, 2024

RAC Pending Button #6435

RAC Pending Button #6435

Conversation

snowystinger commented May 23, 2024 • edited Loading

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

rspbot commented May 23, 2024

rspbot commented Jun 11, 2024

rspbot commented Jun 11, 2024

rspbot commented Jun 11, 2024

rspbot commented Jun 23, 2024

rspbot commented Jun 23, 2024

rspbot commented Jun 30, 2024

rspbot commented Jul 25, 2024

rspbot commented Aug 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rspbot commented Aug 20, 2024

rspbot commented Aug 20, 2024

LFDanLu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rspbot commented Aug 29, 2024

rspbot commented Aug 29, 2024

LFDanLu left a comment

Choose a reason for hiding this comment

rspbot commented Aug 30, 2024

rspbot commented Aug 30, 2024

react-aria-components

/react-aria-components:Button

/react-aria-components:ButtonProps

/react-aria-components:ButtonRenderProps

@react-aria/live-announcer

/@react-aria/live-announcer:announce

@react-spectrum/s2

/@react-spectrum/s2:ActionButton

/@react-spectrum/s2:Button

/@react-spectrum/s2:ActionButtonProps

/@react-spectrum/s2:ButtonProps

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snowystinger commented May 23, 2024 •

edited

Loading