Skip to content

Add isBusy prop to ConfirmDialog #73041

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.

Sign up for GitHub

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

Jump to bottom
Merged
juanfra merged 4 commits into from
Nov 10, 2025
Merged

Add isBusy prop to ConfirmDialog #73041

juanfra merged 4 commits into from
Nov 10, 2025

Conversation

@juanfra
Copy link
Member

@juanfra juanfra commented Nov 6, 2025

What?

Closes #73038

Introducing an isBusy prop to the component. When enabled, the dialog buttons would be disabled, and the confirm button would display a busy state. This would give users a clear visual cue that the action is processing, while still allowing them to dismiss the dialog by clicking outside of it if needed.

Why?

When a ConfirmDialog action takes a while to complete (for example, deleting a large number of records), there’s no indication that the process is underway. This leaves users unsure whether anything is happening. From a UX perspective, adding an isBusy prop will provide clear feedback that the action has started and is still in progress.

How?

Testing Instructions

  1. Checkout this PR.
  2. Run nvm use && npm run storybook:dev
  3. Visit the ConfirmDialog story
  4. Confirm the "With Busy State" works as described.

Screenshots or screencast

confirmdialog-isbusy.mp4

@juanfra juanfra requested a review from ajitbohra as a code owner November 6, 2025 10:32
@juanfra juanfra added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Nov 6, 2025
@github-actions
Copy link

github-actions bot commented Nov 6, 2025 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: juanfra <[email protected]>
Co-authored-by: ntsekouras <[email protected]>
Co-authored-by: aduth <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

ntsekouras
ntsekouras reviewed Nov 7, 2025
View reviewed changes
ntsekouras
ntsekouras reviewed Nov 7, 2025
View reviewed changes
ntsekouras
ntsekouras reviewed Nov 7, 2025
View reviewed changes
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I've left a couple of comments regarding the storybook and tests, but the actual code changes in the component look good.

@ntsekouras ntsekouras requested review from aduth and mirka November 7, 2025 11:45
@juanfra
Copy link
Member Author

juanfra commented Nov 7, 2025

Thanks for reviewing, Nik! I'll wait to see if we keep the tests or not.

aduth
aduth reviewed Nov 7, 2025
View reviewed changes
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Is there an intended use-case for this somewhere in Gutenberg? I'm trying to make sure I understand the need here.

On one hand, I think ConfirmDialog is a high-level component that should surface common customizations of these kinds of modals; on the other, someone still has the option to compose their own Modal in the same way as ConfirmDialog does, and apply these properties to buttons directly. It reminds me of some of the problems raised in #71196, where this is a good example of Modal not always being so easy to customize while ensuring consistency. I think this change could be okay, but maybe in the future we'll want to revisit the Modal abstractions to make it easier to configure buttons in a way that balances developer experience and visual consistency.

As far as the tests are concerned, I think it could be reasonable to have a test, but I'd suggest limiting it to 1 perhaps? For the sake of maintainability and test performance. Optionally rerendering multiple times with different combinations, or moving the "not set busy" assertions into the default rendering test case.

@juanfra
Copy link
Member Author

juanfra commented Nov 10, 2025

Thanks for taking a look and for the thoughtful feedback :)

Is there an intended use-case for this somewhere in Gutenberg? I'm trying to make sure I understand the need here.

I ran into the need while working on a personal project, and it felt like a fairly common interaction pattern for confirm flows (especially when an action may take a bit longer). Maybe DataViews will eventually need something like this.

I believe that if we want the ecosystem to stay aligned with the design direction of the admin, having flexible, open components helps a lot (I believe if certain components are meant to be used only within gutenberg core, we can always make that explicit by marking them as private).

someone still has the option to compose their own Modal in the same way as ConfirmDialog does, and apply these properties to buttons directly.

Totally! composing your own modal is always an option. At the same time, having ready-to-use, extensible components makes things much smoother for plugin authors and extenders and reduces the amount of duplicated UI logic across the ecosystem.

As far as the tests are concerned, I think it could be reasonable to have a test, but I'd suggest limiting it to 1 perhaps? For the sake of maintainability and test performance. Optionally rerendering multiple times with different combinations, or moving the "not set busy" assertions into the default rendering test case.

Sounds good. I'll update the PR.

ntsekouras
ntsekouras approved these changes Nov 10, 2025
View reviewed changes
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Personally I think it's fine to add this prop to this component to enable 3rd party devs (and most probably in GB in the future) to avoid creating custom confirm dialogs with Modal. I'd appreciate a confirmation from @aduth before landing though, as he's more into the components package.

Thanks!

@github-actions
Copy link

github-actions bot commented Nov 10, 2025

Flaky tests detected in e9c7ca0.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19225709746
📝 Reported issues:

@aduth
Copy link
Member

aduth commented Nov 10, 2025

No blockers from me 👍

@juanfra juanfra merged commit d2dc509 into trunk Nov 10, 2025
35 checks passed
@juanfra juanfra deleted the try/73038-add-isbusy-confirmdialog branch November 10, 2025 20:24
@juanfra
Copy link
Member Author

juanfra commented Nov 10, 2025

Thank you 🙌

@github-actions github-actions bot added this to the Gutenberg 22.1 milestone Nov 10, 2025
@arthur791004 arthur791004 mentioned this pull request Nov 28, 2025
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduth aduth aduth left review comments

@ntsekouras ntsekouras ntsekouras approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@mirka mirka Awaiting requested review from mirka

Assignees

No one assigned

Labels

[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Projects

None yet

Milestone

Gutenberg 22.1

Development

Successfully merging this pull request may close these issues.

Add isBusy prop to ConfirmDialog

4 participants

@juanfra @aduth @ntsekouras