WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#26758 closed enhancement (fixed)

Edit Tags form on submission does not stay at the same page and gets redirected

Reported by: chiragswadia Owned by: wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 0.71
Component: Taxonomy Keywords: has-patch dev-feedback
Focuses: ui, accessibility Cc:

Description

Whenever a category / tag is updated ( name / slug / description ), It gets redirected to the tags list ( i.e. edit-tags.php page ).

This is quite annoying and for re-checking the value updated, We have to search the tag in the list and then confirm our value updated.

The MAJOR problem arises when we have custom category/tag meta in addition to the name, slug and description. To verify whether it is updated or not, It has to be re-opened in edit mode after searching from the list.

I have fixed this issue. After updation a proper message is displayed and the redirect to the tags/categories list is prevented.

Attachments (8)

tag_edit_form_redirect.diff (2.2 KB) - added by chiragswadia 6 years ago.
Patch for the enhancement/bug
26758-edit-tag-form.php.patch (2.1 KB) - added by UmeshSingla 6 years ago.
Added Message array and code to display message
26758-edit-tags.php.patch (431 bytes) - added by UmeshSingla 6 years ago.
Checks for referer instead of original referer for 'edittag' case
edit_tags_merged_changes.patch (2.7 KB) - added by chiragswadia 5 years ago.
Added "Go back" link
26758-040714.patch (3.1 KB) - added by rhyswynne 5 years ago.
Part patch (fixes some of comment 18)
26758-040714-2.patch (3.1 KB) - added by rhyswynne 5 years ago.
Reverts "Go To Tags" back to "Go Back" on 26758-040714.patch
26758.patch (4.3 KB) - added by afercia 5 years ago.
26758.2.patch (5.1 KB) - added by afercia 4 years ago.

Download all attachments as: .zip

Change History (41)

@chiragswadia
6 years ago

Patch for the enhancement/bug

#1 @chiragswadia
6 years ago

  • Keywords has-patch needs-testing added

#2 @chiragswadia
6 years ago

  • Version changed from trunk to 3.8

@UmeshSingla
6 years ago

Added Message array and code to display message

@UmeshSingla
6 years ago

Checks for referer instead of original referer for 'edittag' case

#3 @UmeshSingla
6 years ago

I face the same issue, as quite often site requires an additional field for Category, Taxonomy, Tags, and in case of editing terms the form redirects from edit page to Listing page which is incorrect as per the normal UI behaviour, instead the form should redirect to same page.

In edit-tags.php, instead of using wp_get_original_referer, it should be wp_get_referer for the case edittag.

Also various message terms need to be added to edit-tag-form.php.

#4 @UmeshSingla
6 years ago

  • Type changed from enhancement to defect (bug)

#5 @UmeshSingla
6 years ago

  • Keywords dev-feedback added

#6 @UmeshSingla
6 years ago

  • Keywords dev-feedback removed

#7 @UmeshSingla
6 years ago

  • Focuses accessibility added

Any updates over it?

#8 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 3.9

#9 follow-up: @SergeyBiryukov
6 years ago

  • Version changed from 3.8 to 0.71

Has been this way since 0.71.

#10 in reply to: ↑ 9 @UmeshSingla
6 years ago

Replying to SergeyBiryukov:

Has been this way since 0.71.

I think the current behaviour needs a change, will be great if I can have your reviews here or some other developer feedback.

It is the expected behaviour that if I'm saving any form, it redirects to itself, displaying the changes made to it(even WordPress follows it everywhere), then why exception here?

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

#11 @rhyswynne
6 years ago

Patch tested and works as expected (Google Chrome on Windows).

Do suggest that if we go down this route that some sort of notification is presented if we change the UX on this. Checked other functionality (such as Edit Post, Edit Pages etc.) and these are all working when you save they don't return to main page.

#12 @nacin
5 years ago

  • Milestone changed from 3.9 to Future Release

I agree this should change. What I'd like to consider is a back button of some sort. For example, when you edit a user, you get a "Back to Users" link.

This has been this way forever; let's see if we can clean up these patches, merge them into one, and ship it in 4.0.

#13 @UmeshSingla
5 years ago

Agree with your suggestion, but I guess after targetting it to a future release, It is never gonna make it. Happens with many of the tickets.

Still thanks for considering. :)

#14 @samuelsidler
5 years ago

  • Keywords 4.0-early added

@UmeshSingla: Definitely work up a merged patch and clean things up as @nacin recommends. We could get this early in the 4.0-cycle if it's ready.

#15 @chiragswadia
5 years ago

@nacin Its good to see that this will change in the near future, If not 3.9 then 4.0 maybe. And yes what you said about "Go Back" button is definitely required. I have combined the above patch and added the go back link.

@chiragswadia
5 years ago

Added "Go back" link

#16 @chiragswadia
5 years ago

Any updates on this ? I guess this was to be added in early 4.0 release.

#17 @ocean90
5 years ago

  • Milestone changed from Future Release to 4.0
  • Owner set to ocean90
  • Status changed from new to reviewing
  • Type changed from defect (bug) to enhancement

#18 @ocean90
5 years ago

Hello chiragswadia, thanks for the patch. Some things you should try to fix:

  • "Go Back" can't be translated. Take a look at tags/3.9/src/wp-admin/user-edit.php#L204 to see how it works. You should also use the same string to be consistent. Not sure what we can use instead of "Users" here, any idea?. Move the string also to the next line, same as on user-edit.php
  • The back link should be escaped via esc_url().
  • "Go back" will not respect the last page where I was, for example I was on page 2, but the back link is always to the first page. Take a look at tags/3.9/src/wp-admin/includes/class-wp-users-list-table.php#L354 for inspiration.

#19 @helen
5 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

Needs a new patch per ocean90's comment in the next couple of days or will be punted from 4.0.

@rhyswynne
5 years ago

Part patch (fixes some of comment 18)

@rhyswynne
5 years ago

Reverts "Go To Tags" back to "Go Back" on 26758-040714.patch

#20 @rhyswynne
5 years ago

Added an esc_url() around $original_referrer_url and made "Go Back" Translatable.

Struggled a bit with the third issue ocean90 suggested, but added an if statement should $original_referrer_url be not present (i.e. URL accessed directly), the link will not be placed.

#21 follow-up: @DrewAPicture
5 years ago

  • Keywords punt added

There seems to be a consensus on wanting this change, though it doesn't seem like there's a consensus on the best way to go about it. I'd caution us against saddling ourselves with a whole new set of messaging arrays without careful consideration of future maintenance. Suggest punt.

#22 in reply to: ↑ 21 @chiragswadia
5 years ago

Replying to DrewAPicture:

There seems to be a consensus on wanting this change, though it doesn't seem like there's a consensus on the best way to go about it. I'd caution us against saddling ourselves with a whole new set of messaging arrays without careful consideration of future maintenance. Suggest punt.

What you are saying is absolutely correct that having the same message arrays at multiple pages will cause maintenance problems if you consider future releases.

But we cannot rule out the need for this functionality as it`s important in terms of accessibility and user experience.

#23 @ocean90
5 years ago

  • Keywords has-patch needs-refresh added; needs-patch punt removed
  • Milestone changed from 4.0 to Future Release

Patch needs a whitespace/braces check. We're also in beta already. Punt.

#24 @afercia
5 years ago

  • Keywords dev-feedback added; 4.0-early needs-refresh removed

Would be nice to give new life to this ticket. I'm trying :) Refreshed patch:

  • uses wp_reset_vars() as done in user-edit.php, not sure about this
  • tries to manage the case when a term is created via AJAX
  • "Back" link rebuilt to be consistent with "Back to Users" link, uses $tax->labels->name

@afercia
5 years ago

#25 @SergeyBiryukov
4 years ago

#32780 was marked as a duplicate.

#26 @SergeyBiryukov
4 years ago

  • Milestone changed from Future Release to 4.4

This ticket was mentioned in Slack in #core by sergey. View the logs.


4 years ago

#28 @SergeyBiryukov
4 years ago

  • Keywords needs-refresh added
  • Owner changed from ocean90 to afercia

@afercia
4 years ago

#29 @afercia
4 years ago

  • Focuses ui added
  • Keywords needs-refresh removed

Refreshed patch. Added the referer parameter to the row-actions "Edit" link. There's a bit of repetition in the code, might need some cleaning. Visually, the "Back" link would look like this:

https://cldup.com/md-m8B_t3t.png

And for custom taxonomies:

https://cldup.com/rUMvBl1kB6.png

That's similar to the "Back" link in the Edit User screen:

https://cldup.com/TMiO25iqMR.png

From a UI perspective, I'm not sure having the link in a new line looks so nice, open to suggestions.

#30 @lionhurt
4 years ago

This is Very annoying, i am happy to know there is others has same feel

is there any Estimated time to add this patch to the core

#31 @wonderboymusic
4 years ago

  • Owner changed from afercia to wonderboymusic
  • Status changed from reviewing to accepted

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


4 years ago

#33 @wonderboymusic
4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed
Last edited 4 years ago by SergeyBiryukov (previous) (diff)
Note: See TracTickets for help on using tickets.