WordPress.org

Make WordPress Core

#21196 closed defect (bug) (fixed)

Remove obsolete style for "Delete Permanently" link for attachments

Reported by: SergeyBiryukov Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: UI Keywords: has-patch commit
Focuses: 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 (8)

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

Download all attachments as: .zip

Change History (25)

SergeyBiryukov22 months ago

SergeyBiryukov22 months ago

SergeyBiryukov22 months ago

comment:1 SergeyBiryukov22 months ago

  • Keywords ui-feedback added

21196.2.patch 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.

comment:3 lessbloat22 months ago

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.

SergeyBiryukov22 months ago

SergeyBiryukov22 months ago

comment:4 follow-up: SergeyBiryukov22 months ago

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

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

comment:5 in reply to: ↑ 4 lessbloat22 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: SergeyBiryukov22 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: lessbloat22 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: SergeyBiryukov22 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 lessbloat21 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).

comment:10 SergeyBiryukov21 months ago

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

comment:11 lessbloat21 months ago

Yep, 21196.3.patch looks good to me.

comment:12 lessbloat20 months ago

  • Keywords ui-feedback removed

SergeyBiryukov20 months ago

Same as 21196.3.patch, refreshed after [21592]

comment:13 helenyhou20 months ago

  • Keywords commit added

comment:14 bpetty19 months ago

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.

comment:15 SergeyBiryukov19 months ago

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 is still relevant.

comment:16 SergeyBiryukov18 months ago

  • Summary changed from "Delete Permanently" link for attachments is misaligned to Remove obsolete style for "Delete Permanently" link for attachments

comment:17 nacin18 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [22281]:

Remove unused delete style. props SergeyBiryukov. fixes #21196.

Note: See TracTickets for help on using tickets.