Make WordPress Core

Opened 6 years ago

Closed 2 years ago

Last modified 2 years ago

#42937 closed defect (bug) (fixed)

Success Message should display on insertion of new category in Taxonomy page

Reported by: manishamakhija's profile manishamakhija Owned by: joedolson's profile joedolson
Milestone: 5.9 Priority: normal
Severity: normal Version: 4.9.1
Component: Taxonomy Keywords: has-patch has-screenshots has-unit-tests input-validation commit
Focuses: ui, accessibility, administration Cc:

Description

When we add new category then apart from displaying a new category at the top, there should also display a success message like 'new category added successfully' same as we have in 'Add new post' Page.
https://www.awesomescreenshot.com/image/3050291/30f92b69674d681274d708ec6d6e472b

https://www.awesomescreenshot.com/image/3050289/1e5164792645bf217b32b93d2f33e2bc

Attachments (13)

42937.patch (2.3 KB) - added by manishamakhija 6 years ago.
post-message.png (28.7 KB) - added by birgire 6 years ago.
Post Update Message (current)
category-message.png (44.4 KB) - added by birgire 6 years ago.
Category Update Message (suggested)
42937.1.patch (2.4 KB) - added by manishamakhija 6 years ago.
translatable message and category link added
42937.2.patch (2.2 KB) - added by dilipbheda 6 years ago.
Coding Standard Improvement
error-in-UI-in-add-term.webm (5.7 MB) - added by manishamakhija 6 years ago.
New term not append in term list after successful insertion
42937.3.diff (2.4 KB) - added by birgire 6 years ago.
category-added-ajax-message-with-42937.3.diff.jpg (36.7 KB) - added by birgire 6 years ago.
42937.4.diff (7.7 KB) - added by birgire 6 years ago.
42937.5.diff (2.4 KB) - added by joedolson 2 years ago.
Refreshed & updated patch
AddTag.php (4.8 KB) - added by joedolson 2 years ago.
Updated unit tests
42937-before.gif (1.2 MB) - added by hellofromTonya 2 years ago.
Test Report: Test before applying PR 1884 patch. No success message. Confirmed reported issue.
42937-test-after-pr1884-applied.gif (1.4 MB) - added by hellofromTonya 2 years ago.
Test Report: Test after applying PR 1884 patch. Success message displays as expected ✅

Change History (52)

#1 @manishamakhija
6 years ago

  • Keywords has-patch needs-unit-tests added

I have added patch for this ticket

#2 @birgire
6 years ago

Hi @manishamakhija, welcome to WordPress trac.

This looks like a good improvement, I recall adding tags/categories and not being sure if it was added or not.

I noticed that the following string in 42937.patch is not translatable:

$message = 'New '. $tax->labels->singular_name . ' created successfully'; 

Suggestion:

$message = sprintf( __( 'New %s created successfully.' ), $tax->labels->singular_name ); 
Last edited 6 years ago by birgire (previous) (diff)

@birgire
6 years ago

Post Update Message (current)

@birgire
6 years ago

Category Update Message (suggested)

#3 @birgire
6 years ago

  • Focuses ui added
  • Keywords has-screenshots added

@manishamakhija I uploaded your screenshots here above, to avoid them being lost if the image hosting changes.

@manishamakhija
6 years ago

translatable message and category link added

#4 @manishamakhija
6 years ago

Thanks @birgire for your suggestions. I've modified the patch.

#5 @chetan200891
6 years ago

  • Focuses administration added

#6 @dilipbheda
6 years ago

I have made changes to existing .patch to improve coding standard.

  1. Whitespace found at end of line
  2. Expected 1 space before/after closing bracket.
  3. if/else/for/while/try blocks should always use braces

@dilipbheda
6 years ago

Coding Standard Improvement

@manishamakhija
6 years ago

New term not append in term list after successful insertion

#7 @manishamakhija
6 years ago

Need for this new enhancement:
In one of my project when I was entering new term in category, it doesn't show at the top/first position though it is added in the backend which displays after page refresh. For more clarification see video I've attached in it. So it's better if we can add success message too.

@birgire
6 years ago

#8 @birgire
6 years ago

I had another look at this and I would suggest we limit the scope of the ticket to the existing "added" messages. We can see the existing messages here:

https://core.trac.wordpress.org/browser/tags/4.9.5/src/wp-admin/includes/edit-tag-messages.php

There are only three variations for "added":

  • Category added.
  • Tag added.
  • Item added.

where the last one is for a taxonomy that's neither a category or a post_tag.

I also think we should have the "added" ajax admin notice dismissable.

Usually that's done via CSS classes like: notice is-dismissible.

But that's not triggered here via ajax. I think a workaround for that is to trigger the 'wp-updates-notice-added' event.

42937.3.diff is a suggested patch that:

  • Adds a message according to the three existing variations, mentioned above.
  • Adds CSS classes to the admin notice.
  • Makes the admin notice dismissable, by triggering the 'wp-updates-notice-added' event.

Todo:

  • Find a way to use the translated strings from the $messages array and make the ajax "added" message filterable by term_updated_messages.

Further:

  • If we want to modify the "added" message, then I think a new ticket would serve it better.
  • I wonder if the "deleted" message should be considered too? (If so then in another ticket).
Last edited 6 years ago by birgire (previous) (diff)

#9 @birgire
6 years ago

42937.4.diff is a suggested way to:

  • Reuse the translated strings from the $messages array (i.e. with index 3).
  • Allow the message to be filterable with term_updated_messages.
  • Add tests within the new Tests_Ajax_AddTag class with methods:
    • test_add_category().
    • test_add_post_tag().
    • test_add_category_should_work_with_message_filtering().
    • test_adding_category_without_capability_should_error().
    • test_adding_existing_category_should_error().


Last edited 6 years ago by birgire (previous) (diff)

@birgire
6 years ago

#10 @birgire
6 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#11 @gounder
6 years ago

I find this issue with all taxonomies including tags, product categories in WooCommerce or Custom Taxonomies. This really gets difficult when you already have a lot of taxonomy and you have to search for where your addition was added.

other than the message stating which category was added I would suggest like when a category gets deleted that row is highlighted in red the newly added taxonomy is highlighter in green.

http://tsara.demo.inkmyweb.com/wp-content/uploads/2018/04/Screen-Shot-2018-04-22-at-10.40.41-PM.png

#12 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#13 @afercia
5 years ago

  • Focuses accessibility added
  • Keywords input-validation added

#15 @afercia
5 years ago

Thanks everyone for this ticket, patch, and feedback. I'd like to propose a further improvement: when a new term gets created, there's no feedback at all for screen readers. A simple wp.a11y.speak() message would be great, using the same already existing strings for categories, tags, and custom taxonomies:

'Category added.' / 'Tag added.' / 'Term added.'

It would be great to add a speak() message also when terms are deleted :)

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


5 years ago

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


5 years ago

#18 @afercia
5 years ago

  • Type changed from enhancement to defect (bug)

Discussed during today's accessibility bug scrub. Agreed to change this ticket from "enhancement" to "bug" as the lack of proper feedback is an accessibility bug.

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


4 years ago

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


4 years ago

#21 @audrasjb
4 years ago

  • Keywords needs-refresh added
  • Milestone changed from 5.3 to Future Release

This ticket is not going to be ready for WP 5.3 since it still needs a new patch and testing.
Moving from 5.3 to Future release.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


3 years ago

#23 @joedolson
3 years ago

  • Milestone changed from Future Release to 5.9
  • Owner changed from SergeyBiryukov to joedolson

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


2 years ago

@joedolson
2 years ago

Refreshed & updated patch

@joedolson
2 years ago

Updated unit tests

#25 @joedolson
2 years ago

  • Keywords needs-refresh removed

Updated this patch:

  • Added wp.a11y.speak on errors and success message
  • Switched from index 3 (Term updated) to index 1 (Term added), as that's the more appropriate message
  • updated unit tests

I don't have a deep familiarity with the WP unit test suite, so somebody should check this over.

#26 follow-up: @joedolson
2 years ago

@hellofromTonya Can you check over these tests? I updated them, but I don't have sufficient comfort with the test suite to be confident.

#27 @joedolson
2 years ago

  • Keywords needs-testing added

#28 in reply to: ↑ 26 @hellofromTonya
2 years ago

Replying to joedolson:

@hellofromTonya Can you check over these tests? I updated them, but I don't have sufficient comfort with the test suite to be confident.

Sure Joe, happy to help. I'll likely combine the patch and tests to create a PR. Then it run in CI against all the supported PHP versions and gain the extra static checks too.

Adding to my TODO.

This ticket was mentioned in PR #1884 on WordPress/wordpress-develop by hellofromtonya.


2 years ago
#29

Trac ticket: https://core.trac.wordpress.org/ticket/42937

#30 @hellofromTonya
2 years ago

@joedolson PR 1884 combines the 2 patches and updates the unit tests. Should be ready.

@hellofromTonya
2 years ago

Test Report: Test before applying PR 1884 patch. No success message. Confirmed reported issue.

@hellofromTonya
2 years ago

Test Report: Test after applying PR 1884 patch. Success message displays as expected ✅

#31 @hellofromTonya
2 years ago

  • Keywords commit added; needs-testing removed

Test Report

Env:

  • OS: macOS Big Sur
  • WordPress: latest trunk (5.9-alpha)
  • Browsers: Chrome, Firefox, Edge, and Safari
  • Plugins: none
  • Theme: TT2
  • localhost: wp-env

Steps

  1. Go to Posts > Categories
  2. Add a new category by typing a name into the Name field
  3. Press enter, return, or "Add New Category" button. Expected behavior:
    • Category is added
    • A success message "Category added" displays
  4. Repeat but attempt to add a category that already exists. Expected behavior:
    • Category is not added
    • An error message displays: "A term with the name provided already exists with this parent."

Test Results

  • Able to reproduce reported issue ❌. See 42937-before.gif.
  • After applying the patch and repeating steps 1-3, success message displays as expected ✅. See 42937-test-after-pr1884-applied.gif.
  • Step 4 displays an error message as expected before and after patch is applied. No regression introduced ✅

Marking for commit.

#32 @joedolson
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 52168:

Media: Featured image modal loads only selected image.

Fix bug introduced in [50829] that caused media modal to only load the selected image. Executes .more() when loading the modal to ensure that the media collection is available.

Props manishamakhija, birgire, dilipbheda, afercia, hellofromTonya.
Fixes #42937.

#33 @joedolson
2 years ago

In 52169:

Commit Standards: Revert [52168] to correct commit message.

Used incorrect commit message..

Follow up to [52168].

See #42937.

#34 @joedolson
2 years ago

In 52170:

Taxonomy: Display update notices when adding terms.

Display notice and announce to screen readers when a new term is added.

Props manishamakhija, birgire, dilipbheda, afercia, hellofromTonya.
Fixes #42937.

This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.


2 years ago

#37 @joedolson
2 years ago

In 52672:

Comments: Only render term update notices on term screens.

Prevent blank notices from appearing when adding custom fields or terms in the post editor.

Props gadhiyaravi, Boniu91, ravipatel, sabernhardt.
Fixes #54955. See #42937.

#38 @peterwilsoncc
2 years ago

In 52728:

Editor: Only render term update notices on term screens.

Prevent blank notices from appearing when adding custom fields or terms in the post editor.

Props gadhiyaravi, Boniu91, ravipatel, sabernhardt, joedolson.
Fixes #54955. See #42937.

#39 @peterwilsoncc
2 years ago

In 53103:

Script loader: Add wp-a11y as dependency of wp-ajax-response.

Ensure wp.a11y.speak() is available when called in wp-ajax-response.

Follow up to [52170].

Props afercia.
Fixes #55544.
See #42937.

Note: See TracTickets for help on using tickets.