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

Wrong notification message shown when an entity is moved to trash #19144

Closed
enriquesanchez opened this issue Dec 14, 2019 · 13 comments · Fixed by #25563 or #55155
Closed

Wrong notification message shown when an entity is moved to trash #19144

enriquesanchez opened this issue Dec 14, 2019 · 13 comments · Fixed by #25563 or #55155
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.

Comments

@enriquesanchez
Copy link
Contributor

enriquesanchez commented Dec 14, 2019

Describe the bug

When moving a post to trash from the Document settings pane in Gutenberg:

Screen Shot 2019-12-13 at 18 36 15

(Image above is a screen shot of the Gutenberg editor with focus set to the "Move to trash button")

VoiceOver announces after successful completion of operation that the post has been reverted to draft:

Screen Shot 2019-12-13 at 18 37 16

(Image above is a screenshot of the Posts section in wp-admin, where the recently deleted post appears to have been moved to trash but the VoiceOver caption is announcing that the post has been reverted to draft)

I expect the announcement to be "Post moved to trash" instead. Also, the just-deleted post does not appear as a draft, as announced, but instead appears in the Trash, as expected.

To reproduce
Steps to reproduce the behavior:

  1. Open a published post.
  2. On the Document settings pane, click on "Move to trash"
  3. Post is correctly moved to trash, but VoiceOver announces that it has been reverted to draft instead.

Desktop

  • OS: macOS 10.15.1
  • Browser: Safari 13.0.3
@enriquesanchez enriquesanchez added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Testing Needs further testing to be confirmed. labels Dec 14, 2019
@enriquesanchez
Copy link
Contributor Author

@tellthemachines When you have a moment, would you mind confirming if you can reproduce this? Thank you in advance!

@joedolson
Copy link
Contributor

Using VoiceOver with Safari I could not reproduce this issue on macOX 10.14.5.

@enriquesanchez
Copy link
Contributor Author

enriquesanchez commented Jan 7, 2020

I was able to reproduce this again. It appears that there are several consecutive messages that VoiceOver announces really quick that "Post reverted to draft" gets omitted.

I changed VO's verbosity settings to "Low" and I was able to see and hear "Post reverted to draft" for a split second before I get navigated to the Post's list page.

@enriquesanchez enriquesanchez added [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later Needs Accessibility Feedback Need input from accessibility labels Jan 7, 2020
@talldan
Copy link
Contributor

talldan commented Jun 11, 2020

@enriquesanchez I was also able to reproduce the issue at the second time of trying, it seems a little inconsistent, as you say, probably related to the message being interrupted as the page and focus changes.

@talldan talldan added [Type] Bug An existing feature does not function as intended and removed Needs Testing Needs further testing to be confirmed. labels Jun 11, 2020
@enriquesanchez
Copy link
Contributor Author

Thanks for testing @talldan!

I wonder if it's possible to enqueue the announcements so none gets interrupted?

@ntsekouras
Copy link
Contributor

ntsekouras commented Sep 23, 2020

Actually this is because we don't handle the trash case and VoiceOver announces another message from the created notification.

@ntsekouras ntsekouras changed the title VoiceOver announces post has been moved to drafts when it's been moved to trash Wrong notification message shown when an entity is moved to trash Sep 23, 2020
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Sep 23, 2020
@afercia
Copy link
Contributor

afercia commented Sep 23, 2020

Looking alsto at #25563, I couldn't reproduce this. Maybe it happens only when the "redirect" to the Posts page is very slow so that the screen reader has time to announce the aria-live message delivered on the block editor page.

However, I'm not sure why the aria-live message should be delivered on the block editor page in the first place. When trashing a post, users are brought to the Posts page so they're (rightfully) forced to abandon the page where the aria-live message is supposed to work.

Instead, I'd tend to think the final trashing confirmation step is on the Posts page, where an admin notice is already displayed:

Screenshot 2020-09-23 at 19 42 38

To improve accessibility I wouldn't try to make the message on the block editor page "work". Instead I'd try to:

  • improve the accessibility of the admin notice displayed on the Posts page, see https://core.trac.wordpress.org/ticket/50486
  • make sure that when trashing, no aria-live messages are used on the block editor page, as in: prevent the message "Post reverted to draft." (and any other message) to be triggered

@ntsekouras
Copy link
Contributor

Maybe it happens only when the "redirect" to the Posts page is very slow so that the screen reader has time to announce the aria-live message delivered on the block editor page.

It happens after successful deletion of the entity and right before redirecting.

For now the respective PR fixed the case of showing proper message and the current issue.

improve the accessibility of the admin notice displayed on the Posts page, see https://core.trac.wordpress.org/ticket/50486

This is something to work on core and is not part of GB. Thanks for having opened that ticket @afercia.

@afercia
Copy link
Contributor

afercia commented Sep 24, 2020

Thanks @ntsekouras

It happens after successful deletion of the entity and right before redirecting.

My point is that I can't reproduce this issue 🙂 I would have appreciated more details instructions to reproduce it.

Also, I think my previous point sill stands: why we would want to use an aria-live message on a page where users are redirected away from?

@ntsekouras
Copy link
Contributor

My point is that I can't reproduce this issue 🙂 I would have appreciated more details instructions to reproduce it.

I don't know why, but I could see the notification every time, before redirecting :). And this could probably be the case for more users when using a live site and redirecting to something other than localhost (internet connections come in play).

Also, I think my previous point sill stands: why we would want to use an aria-live message on a page where users are redirected away from?

I'm not sure about this from accessibility perspective, but in cases like mine (always seeing the message), I think this is definitely an improvement, being notified properly for my action..

@afercia
Copy link
Contributor

afercia commented Sep 24, 2020

Fair enough 🙂 @ntsekouras can you please have a look at my comment on the PR? The message string is not translatable.

@swissspidy swissspidy reopened this Sep 24, 2020
@swissspidy
Copy link
Member

swissspidy commented Sep 24, 2020

#25563 definitely needs a follow-up to fix the translation issue @afercia mentioned.

Edit: just saw #25604

Very easy but not ideal solution:

noticeMessage = sprintf(
	/* translators: %s: post type singular name. */
	__( '%s trashed.' ),
	postType.labels.singular_name
);

Better solution:

noticeMessage = postType.labels.item_trashed; // where item_trashed is "Post trashed." or "Page trashed."

Filter post type labels to add needed label as long as https://core.trac.wordpress.org/ticket/51387 is pending

There's some precedence here:

gutenberg/lib/compat.php

Lines 473 to 510 in 5de94bb

/**
* Override post type labels for Reusable Block custom post type.
*
* This shim can be removed when the Gutenberg plugin requires a WordPress
* version that has the ticket below.
*
* @see https://core.trac.wordpress.org/ticket/50755
*
* @since 8.6.0
*
* @return array Array of new labels for Reusable Block post type.
*/
function gutenberg_override_reusable_block_post_type_labels() {
return array(
'name' => _x( 'Reusable Blocks', 'post type general name', 'gutenberg' ),
'singular_name' => _x( 'Reusable Block', 'post type singular name', 'gutenberg' ),
'menu_name' => _x( 'Reusable Blocks', 'admin menu', 'gutenberg' ),
'name_admin_bar' => _x( 'Reusable Block', 'add new on admin bar', 'gutenberg' ),
'add_new' => _x( 'Add New', 'Reusable Block', 'gutenberg' ),
'add_new_item' => __( 'Add New Reusable Block', 'gutenberg' ),
'new_item' => __( 'New Reusable Block', 'gutenberg' ),
'edit_item' => __( 'Edit Reusable Block', 'gutenberg' ),
'view_item' => __( 'View Reusable Block', 'gutenberg' ),
'all_items' => __( 'All Reusable Blocks', 'gutenberg' ),
'search_items' => __( 'Search Reusable Blocks', 'gutenberg' ),
'not_found' => __( 'No reusable blocks found.', 'gutenberg' ),
'not_found_in_trash' => __( 'No reusable blocks found in Trash.', 'gutenberg' ),
'filter_items_list' => __( 'Filter reusable blocks list', 'gutenberg' ),
'items_list_navigation' => __( 'Reusable Blocks list navigation', 'gutenberg' ),
'items_list' => __( 'Reusable Blocks list', 'gutenberg' ),
'item_published' => __( 'Reusable Block published.', 'gutenberg' ),
'item_published_privately' => __( 'Reusable Block published privately.', 'gutenberg' ),
'item_reverted_to_draft' => __( 'Reusable Block reverted to draft.', 'gutenberg' ),
'item_scheduled' => __( 'Reusable Block scheduled.', 'gutenberg' ),
'item_updated' => __( 'Reusable Block updated.', 'gutenberg' ),
);
}
add_filter( 'post_type_labels_wp_block', 'gutenberg_override_reusable_block_post_type_labels', 10, 0 );

@skorasaurus skorasaurus added [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked. and removed [Status] In Progress Tracking issues with work in progress labels Feb 18, 2021
pento pushed a commit to WordPress/wordpress-develop that referenced this issue Jun 15, 2023
This allows the block editor to announce the correct message when an entity is moved to the Trash.

Original issue from Gutenberg repository:
* [WordPress/gutenberg#19144 #19144 Wrong notification message shown when an entity is moved to trash]

Props james-roberts, ntsekouras, nrqsnchz, afercia, swissspidy, joedolson, talldanwp, SergeyBiryukov.
Fixes #51387.

git-svn-id: https://develop.svn.wordpress.org/trunk@55923 602fd350-edb4-49c9-b593-d223f7449a82
@SergeyBiryukov
Copy link
Member

The item_trashed post type label is available in WP 6.3 as of r55923, so this should now be unblocked.

markjaquith pushed a commit to WordPress/WordPress that referenced this issue Jun 15, 2023
This allows the block editor to announce the correct message when an entity is moved to the Trash.

Original issue from Gutenberg repository:
* [WordPress/gutenberg#19144 #19144 Wrong notification message shown when an entity is moved to trash]

Props james-roberts, ntsekouras, nrqsnchz, afercia, swissspidy, joedolson, talldanwp, SergeyBiryukov.
Fixes #51387.
Built from https://develop.svn.wordpress.org/trunk@55923


git-svn-id: http://core.svn.wordpress.org/trunk@55435 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this issue Jun 15, 2023
This allows the block editor to announce the correct message when an entity is moved to the Trash.

Original issue from Gutenberg repository:
* [WordPress/gutenberg#19144 #19144 Wrong notification message shown when an entity is moved to trash]

Props james-roberts, ntsekouras, nrqsnchz, afercia, swissspidy, joedolson, talldanwp, SergeyBiryukov.
Fixes #51387.
Built from https://develop.svn.wordpress.org/trunk@55923


git-svn-id: https://core.svn.wordpress.org/trunk@55435 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.
Projects
None yet
8 participants