WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 13 days ago

#46830 accepted defect (bug)

When menu item removed form Customizer menu uncheck it's source item

Reported by: garrett-eclipse Owned by: audrasjb
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-patch has-screenshots
Focuses: ui Cc:

Description

Hello,

When using the Customizer menu the menu items that you add get checked off on the original panel. But when you remove those pages from the menu those items stay checked which can mislead users to think their page is still in the menu. It would be nice if once the last page for the item is removed from the menu it's relevant source menu item be unchecked.

Sorry I hope that wasn't confusing adding a screenshot.

Attachments (5)

Screen Shot 2019-04-08 at 1.37.43 AM.png (62.0 KB) - added by garrett-eclipse 5 months ago.
Added page is checked off
Screen Shot 2019-04-08 at 1.37.52 AM.png (80.2 KB) - added by garrett-eclipse 5 months ago.
Removed page leaves item checked off which is misleading and gives it almost a disabled state
46830.diff (2.6 KB) - added by donmhico 5 weeks ago.
5f08acd6255c1464847595ab922ae4d1.gif (411.8 KB) - added by garrett-eclipse 2 weeks ago.
Successful testing demonstration showing checks revert to + plus icons when all items removed.
46830.2.diff (2.6 KB) - added by donmhico 13 days ago.
Changed @see to @link.

Download all attachments as: .zip

Change History (10)

@garrett-eclipse
5 months ago

Added page is checked off

@garrett-eclipse
5 months ago

Removed page leaves item checked off which is misleading and gives it almost a disabled state

#1 @audrasjb
5 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to audrasjb
  • Status changed from new to accepted

Hi and thanks for the ticket @garrett-eclipse

I can reproduce the issue on my side. Moving this one to the next release.

@donmhico
5 weeks ago

#2 @donmhico
5 weeks ago

  • Keywords has-patch added; needs-patch removed

@garrett-eclipse @audrasjb - If you have time, can you test and review my attached patch 46830.diff.

Thanks.

@garrett-eclipse
2 weeks ago

Successful testing demonstration showing checks revert to + plus icons when all items removed.

#3 @garrett-eclipse
2 weeks ago

  • Keywords has-screenshots added

Thanks @donmhico this worked really nicely, testing both Posts and Pages the check indicator reverts to the + add icon once all items are removed as desired.

Reviewing the code everything looked very well written thank you. In php unit tests I've seen ticket references as @ticket XXXXX but I'm not sure if that's also the standard with JS (referring to your @see reference).

@audrasjb would you mind taking a final review this works nicely and applies cleanly. I feel it's ready to commit but Customizer is your component ;)

P.S. While testing I found the check icon is lost if you click 'Add Items' to open/close the drawer. I've opened 47990 to look into keeping that check mark persistent.

Last edited 2 weeks ago by garrett-eclipse (previous) (diff)

#4 @donmhico
2 weeks ago

Thanks @garrett-eclipse. Maybe i'll use @link instead.

@see: A function or class relied on.
@link: URL that provides more information.

https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/javascript/

@donmhico
13 days ago

Changed @see to @link.

#5 @garrett-eclipse
13 days ago

Nice, good call thanks @donmhico

Note: See TracTickets for help on using tickets.