WordPress.org

Make WordPress Core

#47173 closed defect (bug) (fixed)

Nav Menus: Delete Menu action link is misaligned in responsive view

Reported by: ianbelanger Owned by: audrasjb
Milestone: 5.2.1 Priority: normal
Severity: normal Version: 4.9
Component: Menus Keywords: has-patch has-screenshots fixed-major
Focuses: Cc:

Description (last modified by ianbelanger)

When adjusting line heights to unitless values, I noticed that the Delete Menu action link on the Menus page is misaligned when the screen is under 782px. Screenshot and patch coming

Attachments (8)

47173-delete-menu-action-misaligned-responsive-view.PNG (10.8 KB) - added by ianbelanger 14 months ago.
Before applying patch
47173.diff (366 bytes) - added by ianbelanger 14 months ago.
adds padding-top: 1em; to delete menu action
47173-delete-menu-action-misaligned-responsive-view-after.PNG (10.8 KB) - added by ianbelanger 14 months ago.
After applying patch
Remove top padding from publish button.png (32.6 KB) - added by mukesh27 14 months ago.
47173.1.diff (417 bytes) - added by ianbelanger 14 months ago.
Updates patch to remove publishing action top padding
47173.2.diff (422 bytes) - added by ianbelanger 14 months ago.
Updates patch to only target top publishing action with the top padding
47173-after-patch.PNG (27.2 KB) - added by ianbelanger 14 months ago.
After applying 47173.2.diff
4.8-trunk.png (16.0 KB) - added by desrosj 14 months ago.

Download all attachments as: .zip

Change History (24)

@ianbelanger
14 months ago

adds padding-top: 1em; to delete menu action

#1 @ianbelanger
14 months ago

  • Keywords has-patch has-screenshots needs-testing added

#2 @audrasjb
14 months ago

  • Milestone changed from Awaiting Review to 5.2.1
  • Owner set to audrasjb
  • Status changed from new to reviewing

#3 @audrasjb
14 months ago

Hi,
Thanks for the patch, I'll get this one reviewed soon :-)

#4 @audrasjb
14 months ago

  • Keywords needs-testing removed
  • Status changed from reviewing to accepted

I tested 47173.diff, it applies well and it fixed the issue on my side.
This should be fixed in 5.2.1 since it's a regression coming from 5.2.

Thanks @ianbelanger :-)

#5 @ianbelanger
14 months ago

Thanks @audrasjb, although I don't think that this issue was introduced in 5.2. I am seeing it in 5.1.1 as well, but it wouldn't hurt to get it in 5.2.1. Thanks for the quick review!

#6 @ianbelanger
14 months ago

  • Description modified (diff)

#7 @mukesh27
14 months ago

@ianbelanger instead of adding top padding can we remove top padding from publish button? check attached screenshot after remove padding from publish button.

https://core.trac.wordpress.org/browser/trunk/src/wp-admin/css/nav-menus.css#L838

.nav-menus-php .major-publishing-actions .publishing-action {
	padding-top: 1em;
}

#8 @garrett-eclipse
14 months ago

  • Keywords needs-refresh added

Thanks @ianbelanger reviewing the patch and screenshot @mukesh27 provided I agree removing the top padding provides a nicer presentation as it gives the action a vertical centering in the space with equal padding above/below and to the right of the action. With that in mind going to add a refresh.

@ianbelanger
14 months ago

Updates patch to remove publishing action top padding

#9 @ianbelanger
14 months ago

  • Keywords needs-refresh removed

#10 @afercia
14 months ago

  • Keywords needs-refresh added
  • Version set to 4.9

I think this was introduced in WordPress 4.9 see [41197] / #41499.

The top padding should stay for the "Save Menu" at the top of the page but shouldn't target also the "Save Menu" at the bottom.

#11 @desrosj
14 months ago

  • Keywords needs-design-feedback added
  • Milestone changed from 5.2.1 to 5.2.2

This still needs a refresh, and I'd love for it to get some design attention. Going to move it to 5.2.2.

@ianbelanger
14 months ago

Updates patch to only target top publishing action with the top padding

#12 @ianbelanger
14 months ago

  • Keywords commit added; needs-refresh needs-design-feedback removed
  • Milestone changed from 5.2.2 to 5.2.1

I was actually working on a fix for this when you changed the milestone @desrosj. Just uploaded the patch to fix the issue. Not sure that we need design-feedback since it is a regression from 4.9, but if someone from design wants to weigh-in that would be great. Going to move back into 5.2.1 milestone and mark for commit.

@ianbelanger
14 months ago

After applying 47173.2.diff

@desrosj
14 months ago

#13 @desrosj
14 months ago

The left of my screenshot is 4.8 and the right is with 47173.2.diff. Since this is a regression, let's get this in.

#14 @desrosj
14 months ago

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

In 45324:

Menus: Fix alignment of menu actions on mobile.

In [41197], changes were made to improve the responsive view for the Menu Name field and Save buttons. This caused the Delete Menu button to be misaligned.

Props ianbelanger, audrasjb, afercia, mukesh27.
Fixes #47173.

#15 @desrosj
14 months ago

  • Keywords fixed-major added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport.

#16 @desrosj
14 months ago

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

In 45325:

Menus: Fix alignment of menu actions on mobile.

In [41197], changes were made to improve the responsive view for the Menu Name field and Save buttons. This caused the Delete Menu button to be misaligned.

Merges [45324] to the 5.2 branch.

Props ianbelanger, audrasjb, afercia, mukesh27.
Fixes #47173.

Note: See TracTickets for help on using tickets.