Add loading prop for Button and IconButton
#3582
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: a7b114b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
langermank
temporarily deployed
to
github-pages
GitHub Actions
Inactive
langermank
temporarily deployed
to
github-pages
GitHub Actions
Inactive
langermank
temporarily deployed
to
github-pages
GitHub Actions
Inactive
langermank
temporarily deployed
to
github-pages
GitHub Actions
Inactive
langermank
changed the title
loading prop for Button and IconButton
mperrotti
approved these changes
Mar 15, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Merged
13 tasks
13 tasks
siddharthkp
added a commit
that referenced
this pull request
Apr 4, 2024
This was referenced Apr 4, 2024
Closed
siddharthkp
added a commit
that referenced
this pull request
Apr 10, 2024
7 tasks
lukasoppermann
pushed a commit
that referenced
this pull request
Apr 16, 2024
* draft loading state * cleanup * add story to trigger loading * merge * icon button * Create lazy-jobs-pump.md * add prop for loading message * handle no visuals loading state * updates snapshots after merging from main * uses unique ID for loading messages, preserves aria-describedby passed to button, rms aria-busy * adds Storybook examples for btn loading error message * reverts unintentional default link underlining * changes loadingMessage to loadingAnnouncement * updates draft Button component with loading prop * updates legacy button counter behavior when loading * Revert "updates draft Button component with loading prop" This reverts commit 7f7f326. * moves error behavior stories to 'Examples' section * screenreader fixes * adds and updates unit tests * re-updates snapshots after using correct VisuallyHidden * documents loading props * adds VRTs, updates loading feature stories * simplifies inner visual/spinner rendering logic * removes example stories (we can put them back when Flash supports focusing its heading) * excludes loading buttons from axe contrast check * fixes visual regression: button counter vertical alignment * prevents double spinners when leading and trailing visuals are passed * test(e2e): update story ids * test(vrt): update snapshots * test(e2e): disable animations in screenshots * test(vrt): update snapshots * preserves rest state styles when button is loading * adds story for success and error announcement * test(vrt): update snapshots * fixes ButtonGroup regression * delete broken snapshots * also targets anchor tags in ButtonGroup styles * add conditional wrapper * fix group * test(vrt): update snapshots * lint * fixes unit tests, updates snapshots * Update src/Button/Button.docs.json Co-authored-by: Pavithra Kodmad <pksjce@github.com> * fixes 'block' layout for loading buttons * uses new internal Status component for loading announcement * updates Tooltip V2 tests to account for loading messageID in button's aria-describedby * fixes BoxProps type import to new preferred syntax * appease the linter * rms ConditionalWrapper * revert back to using ConditionalWrapper with aria-describedby * test(vrt): update snapshots --------- Co-authored-by: Mike Perrotti <mperrotti@github.com> Co-authored-by: Josh Black <joshblack@users.noreply.github.com> Co-authored-by: langermank <langermank@users.noreply.github.com> Co-authored-by: Pavithra Kodmad <pksjce@github.com> Co-authored-by: mperrotti <mperrotti@users.noreply.github.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 25, 2024
…4485) * Revert "Revert "Add `loading` prop for `Button` and `IconButton` (#3582)" (#4…" This reverts commit c01901f. * only overrides btn label when loading * update tooltip tests to accomodate loading message aria-describedby * import missing modules in IconButton stories * makes aria-labelledby prop remain set when when button is in a loading state * Update packages/react/src/Button/Button.examples.stories.tsx Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> * Update packages/react/src/Button/Button.examples.stories.tsx Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> * updates textinput snapshots * addresses remaining PR feedback * appeases the linter, updates snaps * leaves loading prop undefined so we do not always render the wrapper * test(vrt): update snapshots * trying again without changing colors on faux-disabled buttons * replaces Status with AriaStatus * replaces one more Status with AriaStatus * rms added aria-disabled styles * test(vrt): update snapshots * Update icon button tests to make it work with the loading state * add story with leading visual and count * misc bugfixes: - ensures counter stays rendered even when no children are passed - preserves space between elements that are children of span[data-component=text] - adds story and VRT for buttons with a trailing action but no leading/trailing visuals * test(vrt): update snapshots --------- Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com> Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> Co-authored-by: mperrotti <mperrotti@users.noreply.github.com> Co-authored-by: Armagan Ersoz <broccolinisoup@github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a
loadingstate to bothButtonandIconButton. The markup follows the recommended path forward from accessibility design, and mirrors the implementation from theExport CSVbutton as part of GH org security coverage pages.Implementation details
aria-disabledinstead ofdisabledto maintain focus managementNew props:
loading: booleanloadingAnnouncement: default toLoadingor provide a custom message to be announced on ATsCloses https://github.com/github/primer/issues/2680
Screenshots
CleanShot.2023-10-11.at.14.55.13.mp4
CleanShot.2023-10-11.at.14.55.42.mp4
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.