WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#13720 closed defect (bug) (fixed)

Custom header UX improvements

Reported by: nacin Owned by: ocean90
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Administration Keywords: commit i18n-change has-patch
Focuses: Cc:

Description

  1. nonce isn't kicking in after upload/crop. When you come back to the screen, a refresh that resubmits the POST data will crop your cropped image further.
  1. I believe the Crop button should be blue.
  1. We need to validate that an image is actually getting uploaded. It took an HTML file for me and after having me crop an image of a broken image, it then threw errors. We can probably extend the 3.0 commits in #11946.
  1. Clicking "Upload" without choosing a file calls wp_die() but after admin-header is already generated. We should either move that to wp_die() or redirect back with an error message if possible.

We should check custom background for some of these as well.

needs-patch(es). I will work on this a bit but if someone else wants to run with it, be my guest.

Attachments (3)

13720-blue--crop-button.patch (665 bytes) - added by ocean90 5 years ago.
Blue Button
13720-blue-crop-button.patch .patch (670 bytes) - added by ocean90 5 years ago.
13720-entity-cleanup.diff (993 bytes) - added by zeo 5 years ago.
entity cleanup.

Download all attachments as: .zip

Change History (15)

comment:1 @ocean905 years ago

  • Owner set to ocean90
  • Status changed from new to accepted

comment:2 @jane5 years ago

Yes, the crop button should be blue, since it doesn't take you back to the screen to save changes, but just publishes the cropped image, making it a primary submit button.

In the future, I'd prefer for clicking crop to return you to the previous screen so you can see the cropped image in the previewer and then click on update or choose to discard it rather than going immediately live.

comment:3 @ocean905 years ago

jane, that's a good point. This could also fix the problem with the reload (point 1).

  • upload an image
  • crop it (if you want)
    • change button "Crop image" to "Crop image and return back", maybe add also a button "Cancel"
    • positiv: it's not directly live
  • when we redirect back we post the path to the cropped image and fill a hidden input field with the path
  • Save the header settings and the path when you click "Save Changes"


What do you think?

comment:4 @jane5 years ago

I think that when we change the behavior, the button should still say crop, because that's what it's meant to do. If we wanted to differentiate, we should have made it say 'crop & publish' for 3.0 (and we really should have, to set up the proper user expectation and avoid unintentional publishing of headers), but i believe we are in string freeze now.

comment:5 @ocean905 years ago

I would say, because we are in string freeze, already in RC2 and it's not a regression we should only change the button color and the rest in early 3.1.

@ocean905 years ago

Blue Button

comment:6 @nacin5 years ago

Going live immediately with the crop is something I am always forgetting, but it's quite important I feel. Since it seems we're leaving the 3.0 behavior -- which I did not bother to address here -- I would be unopposed to changing the string to "Crop and Publish" and letting the translators know.

comment:7 @nacin5 years ago

  • Keywords i18n-change added

Marking for potential change.

comment:8 @ocean905 years ago

  • Keywords has-patch added

After a discussion with nacin we want only the blue (with the i18n-change?) in 3.0 and the other points in 3.1, because it needs a bigger change to handle better the error messages and redirects.

comment:9 @ryan5 years ago

(In [15158]) Make crop button primary and change text to note that cropping the image publishes it. Props ocean90. see #13720

@zeo5 years ago

entity cleanup.

comment:10 @zeo5 years ago

  • Keywords commit added

comment:11 @nacin5 years ago

(In [15185]) Use entity. props zeo, see #13720.

comment:12 @nacin5 years ago

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

Closing as fixed on 3.0.

Note: See TracTickets for help on using tickets.