Skip to content
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

ConfirmDialog: Replace 'OK' with more descriptive label #60360

Closed
afercia opened this issue Apr 2, 2024 · 6 comments · Fixed by #60364
Closed

ConfirmDialog: Replace 'OK' with more descriptive label #60360

afercia opened this issue Apr 2, 2024 · 6 comments · Fixed by #60364
Assignees
Labels
Needs Design Feedback Needs general design feedback. [Package] Edit Site /packages/edit-site [Package] Editor /packages/editor [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@afercia
Copy link
Contributor

afercia commented Apr 2, 2024

Description

See also #60027

The ConfirmDialog component was introduced in #34153 because of two main purposes:

Quoting from the related issues, see #16583 and #33937

  • The native window.confirm() isn't an ideal experience as we're unable to customise the OK button to have a more descriptive label.
  • Major browsers - starting with Chrome - will soon restrict the alert/confirm/prompt functions from being used from within cross-origin iframes.

Regarding the first point, it appears some instances of the ConfirmDialog just use the default OK button text, thus defeating one of the purposes it was introduced for.

I'd tend to think re-enforcing the in-progress action by using a more meaningul button text would make the user experience better. Also, there's some inconsistency throughout the UI. I'd tend to think all ConfirmDialog should either use 'OK' oar a more specific text.

Some exampke screenshots:

Screenshot 2024-04-02 at 11 49 38

Screenshot 2024-04-02 at 12 12 08

Screenshot 2024-04-02 at 11 46 16

Screenshot 2024-04-02 at 11 50 52

Screenshot 2024-04-02 at 12 13 20

Screenshot 2024-04-02 at 12 19 38

Screenshot 2024-04-02 at 12 21 20 2

Step-by-step reproduction instructions

  • Go through all ConfirmDialogs
  • Observe the inconsistent pattern for the confirm button text.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Package] Editor /packages/editor [Package] Edit Site /packages/edit-site labels Apr 2, 2024
@afercia afercia self-assigned this Apr 2, 2024
@afercia
Copy link
Contributor Author

afercia commented Apr 2, 2024

Worth also noting that the 'Best practices` section added to the component readme in #59825 now recommends to:

Use a descriptive text for the confirm button.

@afercia afercia removed the [Type] Bug An existing feature does not function as intended label Apr 2, 2024
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Apr 2, 2024
@afercia
Copy link
Contributor Author

afercia commented Apr 2, 2024

The ConfirmDialog used for switching to template editing is a special case. To me, its usage isn't ideal in the first place.

A ConfirmDialog should be used 'only for short confirmation messages where a cancel and confirm actions are provided.'.
The copy of this instance doesn't make very clear what the action to confirm is. It onlyy tells used they just did something.

You’ve tried to select a block that is part of a template, which may be used on other posts and pages.

Instead, ideally the copy should ask a question or, at least, the action to confirm should me more clear.

Screenshot:

Screenshot 2024-04-02 at 16 15 33

Update:
I struggled a bit understanding how to make this ConfirmDialog appear. Steps:

  • Go to the Site editor > Pages > edit a page.
  • Double click on an empty area at the side of a block provided by the page template, e.g. the page title.

Originally, the copy of this dialog in #51366 was Edit your template to edit this block..

In #60016 it was changed to You’ve tried to select a block that is part of a template, which may be used on other posts and pages. I do understand why it was changed, as the underlying functionality changed. However, a confirm dialog copy should always ask a question, whether it's an explicit or implicit question. I would like to change this copy, I'll try to think at something meaningful anc concise. Any suggestion would be appreciated Cc @noisysocks

@afercia
Copy link
Contributor Author

afercia commented Apr 3, 2024

What about Would you like to edit the template this block is part of?

@afercia
Copy link
Contributor Author

afercia commented Apr 3, 2024

The copy Any unsaved changes will be lost when you apply this revision. should be changed as well, to ask a question. I'd like to propose:

Are you sure you want to apply this revision? Any unsaved changes will be lost.

@noisysocks
Copy link
Member

noisysocks commented Apr 9, 2024

What about Would you like to edit the template this block is part of?

Thanks for flagging. Works for me. I think we could drop the last part so that it is:

You've tried to select a block that is part of a template, which may be used on other posts and pages. Would you like to edit the template?

[Cancel] [Edit template]

@afercia
Copy link
Contributor Author

afercia commented Apr 10, 2024

Thanks for the feedback, A little verbose but it works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Package] Edit Site /packages/edit-site [Package] Editor /packages/editor [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants