Ticket #21196 (new defect (bug))

Opened 4 months ago

Last modified 6 days ago

Remove obsolete style for "Delete Permanently" link for attachments

Reported by: SergeyBiryukov Owned by:
Priority: normal Milestone: 3.5
Component: UI Version:
Severity: normal Keywords: has-patch commit
Cc:

Description

If EMPTY_TRASH_DAYS is set to 0 (zero) in wp-config.php, "Delete Permanently" link for attachments is misaligned (see the screenshots).

Attachments

21196.delete.png Download (4.8 KB) - added by SergeyBiryukov 4 months ago.
21196.delete-permanently.png Download (5.0 KB) - added by SergeyBiryukov 4 months ago.
21196.patch Download (385 bytes) - added by SergeyBiryukov 4 months ago.
21196.2.patch Download (714 bytes) - added by SergeyBiryukov 4 months ago.
21196.after.png Download (5.0 KB) - added by SergeyBiryukov 4 months ago.
21196.3.patch Download (1.4 KB) - added by SergeyBiryukov 3 months ago.
21196.4.patch Download (4.6 KB) - added by SergeyBiryukov 3 months ago.
21196.5.patch Download (1.3 KB) - added by SergeyBiryukov 8 weeks ago.
Same as 21196.3.patch, refreshed after [21592]

Change History

  • Keywords ui-feedback added

21196.2.patch Download gives "Delete Permanently" link the same 5px padding as "Delete" has (not sure if it's necessary though).

We could also probably unify del-link (Delete) and delete (Delete Permanently) classes.

Good eye! :-)

I'd drop the #media-items a.delete style in wp-admin/css/media.dev.css (like you've done here), but I wouldn't add the new #media-items a.delete style to wp-admin/css/wp-admin.dev.css.

Reason being: the "Use as feature image" link already has a margin-right setting of 20px.

comment:4 follow-up: ↓ 5   SergeyBiryukov3 months ago

Thanks, then it sounds like the 5px padding for .del-link should also be removed (21196.3.patch Download).

21196.4.patch Download is an attempt to also unify .del-link (Delete) and .delete (Delete Permanently & Move to Trash).

comment:5 in reply to: ↑ 4   lessbloat3 months ago

Replying to SergeyBiryukov:

Looks good to me. But you should probably keep the padding-right: 5px; in wp-admin-rtl.dev.css

comment:6 follow-up: ↓ 7   SergeyBiryukov3 months ago

wp-admin-rtl.dev.css should generally mirror the changes in wp-admin.dev.css, so I guess it would make sense to remove it from there as well.

comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8   lessbloat3 months ago

Replying to SergeyBiryukov:

I'm an idiot - please forget my last comment. :-) I tested 21196.4.patch with EN, and AR language settings. Everything looks good, with one minor exception - by removing the ".delete" class, hovering over the link no longer turns the background of the link red.

comment:8 in reply to: ↑ 7 ; follow-up: ↓ 9   SergeyBiryukov3 months ago

Replying to lessbloat:

by removing the ".delete" class, hovering over the link no longer turns the background of the link red.

Yes, but the red background seems inconsistent with the usual "Delete" link (without EMPTY_TRASH_DAYS set to zero). Not sure if the background is necessary.

Having two similar links is confusing though, as mentioned in ticket:21194:2 and ticket:21194:9, since they essentially do the same thing, unless I'm missing something. We could probably unify them as well, but that needs a bit more investigation. Related: ticket:11149:19.

comment:9 in reply to: ↑ 8   lessbloat3 months ago

Replying to SergeyBiryukov:

We could probably unify them as well

As you alluded to, that seems like a whole other discussion/ticket. Personally I like the red background on hover for delete links as it provides a quick visual "Wait do I really want to do this" without requiring a confirmation dialog (but that's just my 2 cents).

Agreed, so 21196.3.patch Download is the way to go then.

Yep, 21196.3.patch looks good to me.

  • Keywords ui-feedback removed

Same as 21196.3.patch, refreshed after [21592]

  • Keywords commit added

I tried to look at confirming this ticket really quickly, but I don't see this behavior in the old media upload/insert dialog (within either Gallery and Media Library tabs). The "Delete Permanently" link appears to be aligned. Maybe I'm looking in the wrong place? (Where I see this looks exactly like the cropped screenshots attached though - the working one)

However, I also wanted to note that it looks like this UI is being replaced entirely by  this maybe? So is this going to be relevant? I couldn't even pull up any dialog or page that contains this UI (that I can find) if I were assuming that the new Beta Media button is only available.

I did checkout r21592 in order to test this though (without beta media), and just didn't see the misalignment.

Tested with Chrome.

The button class was changed from .delete to .delete-permanently in [21504], so the #media-items a.delete selector in media.css no longer applies.

It should be removed as obsolete now, so 21196.5.patch Download is still relevant.

  • Summary changed from "Delete Permanently" link for attachments is misaligned to Remove obsolete style for "Delete Permanently" link for attachments
Note: See TracTickets for help on using tickets.