Skip to content

fix: Busy button should display wait cursor and have aria-busy #93793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

priscilawebdev
Copy link
Member

@priscilawebdev priscilawebdev commented Jun 18, 2025

I was unable to make my cursor appear in my recordings, so unfortunately, there are no before-and-after images.

@priscilawebdev priscilawebdev marked this pull request as ready for review June 18, 2025 09:19
@priscilawebdev priscilawebdev requested a review from a team as a code owner June 18, 2025 09:19
@priscilawebdev priscilawebdev requested a review from a team June 18, 2025 09:19
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 18, 2025
Copy link
Contributor

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@getsentry/design-engineering I would probably like to streamline the API for busy and disabled a bit. Currently its two booleans, but can you really be in both states? We sometimes set both flags:

busy={isAccepting}
disabled={isAccepting}

so which cursor you then get depends on how its implemented ...

@JonasBa
Copy link
Member

JonasBa commented Jun 20, 2025

@getsentry/design-engineering I would probably like to streamline the API for busy and disabled a bit. Currently its two booleans, but can you really be in both states? We sometimes set both flags:

busy={isAccepting}
disabled={isAccepting}

so which cursor you then get depends on how its implemented ...

Let's bring this up to @Jesse-Box. I would like to understand if we can encapsulate busy inside a variant, but I am not sure I understand all the use-cases here.

From what I can tell, busy seems to be very similar to isLoading or something equivalent, and maybe there is something we can do there. Our buttons are very poorly implemented in the sense that they don't really display loading state very well, so maybe we can solve that along with busy.

Aside from that, I think busy is a poorly named prop and an unfamiliar concept in the context of a button, and I suspect that might also be one of the reasons why this might not be used much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants