WordPress.org

Make WordPress Core

Opened 10 months ago

Closed 5 months ago

#36984 closed enhancement (fixed)

Customizer Menus: Show Pages at the top of the list, expanded

Reported by: melchoyce Owned by: ryankienstra
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: has-screenshots has-patch
Focuses: ui, administration Cc:

Description

The majority of people put pages in their menus, if they have them. Currently you're presented with a long list of items you can add to your menu and you need to hunt for "pages." Let's make it easier by listing pages at the top of the Customizer Menu UI, automatically expanded.

Attachments (19)

custom-to-top.diff (691 bytes) - added by ryankienstra 5 months ago.
iphone-5-no-auto-expand.png (2.1 MB) - added by ryankienstra 5 months ago.
samsung-6.png (539.8 KB) - added by ryankienstra 5 months ago.
galaxy-6-pages.png (585.9 KB) - added by ryankienstra 5 months ago.
nexus-6.png (623.0 KB) - added by ryankienstra 5 months ago.
Nexus 6 with "Pages" expanded
mobile.png (252.3 KB) - added by ryankienstra 5 months ago.
after-newest-patch.png (465.6 KB) - added by ryankienstra 5 months ago.
before-new-patch.png (458.9 KB) - added by ryankienstra 5 months ago.
pages-first-expanded.png (1.0 MB) - added by ryankienstra 5 months ago.
pages-before-edits.png (997.8 KB) - added by ryankienstra 5 months ago.
mobile-iphone-6.png (252.3 KB) - added by ryankienstra 5 months ago.
iphone-5.png (2.1 MB) - added by ryankienstra 5 months ago.
iphone-5-no-auto-expand.pdf (2.4 MB) - added by ryankienstra 5 months ago.
iphone-5-no-auto-expand.2.pdf (2.4 MB) - added by ryankienstra 5 months ago.
iphone-5-not-auto-expanded.png (2.2 MB) - added by ryankienstra 5 months ago.
iphone-5-without-auto-expand.png (1.7 MB) - added by ryankienstra 5 months ago.
iphone-5-without-auto-expansion.png (457.2 KB) - added by ryankienstra 5 months ago.
pages-collapsed.png (510.0 KB) - added by ryankienstra 5 months ago.
172.diff (654 bytes) - added by ryankienstra 5 months ago.

Change History (41)

#1 @ryankienstra
6 months ago

In Progress
Commit moves pages to top of new menu items

Hi @melchoyce,
This commit moves the pages to the top of the new menu item UI. But it doesn't expand the accordion yet:

Here's an identical patch.

Last edited 6 months ago by ryankienstra (previous) (diff)

#2 @ryankienstra
6 months ago

Pull Request And Patch

This GitHub pull request and identical patch move the "Pages" to the top of the order of new menu items (screenshot). Before, when clicking to add a new item to a menu in the Customizer, pages were third in the order of available items (screenshot).

Also, the "Pages" section is now open by default. Please see the pull request for comments on the code.

#3 @westonruter
6 months ago

  • Milestone changed from Awaiting Review to Future Release

#4 @westonruter
6 months ago

  • Component changed from Menus to Customize
  • Keywords has-patch commit added
  • Milestone changed from Future Release to 4.7
  • Owner set to westonruter
  • Status changed from new to accepted

#5 @westonruter
6 months ago

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

In 38767:

Customize: Show Pages section first and pre-expanded in list of available nav menu items.

Props ryankienstra, westonruter.
Fixes #36984.

#6 @celloexpressions
6 months ago

  • Keywords needs-patch needs-screenshots needs-user-testing added; has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version set to 4.3

Sorry, this is the first time I've seen this ticket.

Pages should not be separated from the other post type sections by custom links - links probably need to remain above pages for that reason. In other words, all available menu item types should be grouped by object type - custom, post types, and taxonomies. Custom links is functionality different, so it shouldn't be placed in the middle of the other sections.

I'm okay with auto-expanding pages, but we should probably user test this. We have quite a lot of "before" testing, most recently when users played with menus during the customize themes testing, and there is always confusion here. We should verify that this either helps or doesn't make things worse.

We should also add before/after screenshots to the ticket for desktop and mobile. Does auto-expanding make sense on small screens as well?

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

#8 @helen
5 months ago

I think the intent is the right one, though custom links should probably move back up for type-grouping. In my observations, first-time users (or people who haven't had to edit a nav menu in a while, which includes yours truly) don't quite know where to start building or editing a nav menu. I think it is a fairly safe initial assumption that most users are making page-based nav menus, and especially now with being able to add new pages while you're at it, it makes sense to have it expanded by default.

And yes, screenshots please :)

#9 @westonruter
5 months ago

  • Owner changed from westonruter to ryankienstra
  • Status changed from reopened to assigned

@ryankienstra would you add screenshots and add a patch to address the feedback?

#10 @ryankienstra
5 months ago

Patch To Move "Custom Links" above "Pages"

Hi @westonruter,
Here's a patch that moves "Custom Links" above "Pages," as @celloexpressions and @helen suggested.

Here's a desktop screenshot of the patch above, and here's a desktop screenshot of before that patch. Here's a screenshot of the display before any of this ticket's edits.

Here's a mobile screenshot of iPhone 6, and it looks like 3 panels are still visible. Even with "Pages" expanded. I still need to post more mobile screenshots.

This comment now has the links to the images uploaded to Trac, instead of Cloudup. I still need to replace the links in the next comment.

Last edited 5 months ago by ryankienstra (previous) (diff)

#11 @ryankienstra
5 months ago

Mobile Screenshots
"Pages" section auto-expanded vs. not expanded

@westonruter,
Here are some mobile screenshots, with the latest patch applied. These all have the "Pages" section auto-expanded. This displays well on devices of 5 inches and more:

In those cases, there's enough space below the expanded "Pages" section to see that other sections are below, like the "Pages" section.

On a smaller display, like the 4-inch iPhone 5 (screenshot), it's not clear that there are sections under the "Pages" section.

Here are some screenshots without the "Pages" section auto-expanded:

All of the sections are visible, even on the 4-inch iPhone 5.

Last edited 5 months ago by ryankienstra (previous) (diff)

#12 @westonruter
5 months ago

  • Keywords has-screenshots added; needs-screenshots removed

@ryankienstra cheers. Would you upload the screenshots as attachments to Trac itself? This way we won't lose them if CloudUp ever goes away. One of the screenshots for Nexus 6 failed to upload to CloudUp.

#13 @westonruter
5 months ago

In 38794:

Customize: Move Pages below Custom Links in available nav menu items panel.

Props ryankienstra, celloexpressions.
See #36984.

@ryankienstra
5 months ago

Nexus 6 with "Pages" expanded

@ryankienstra
5 months ago

#14 @celloexpressions
5 months ago

Thanks for all of the screenshots @ryankienstra!

I found a bug that I think is caused by this ticket. The Pages section now collapses when loading if you open it before it's loaded the initial items, and it also collapses whenever an infinite-scroll load happens.

#15 @ryankienstra
5 months ago

Uploaded Images To Trac

Hi @westonruter,
In response to your comment:

@ryankienstra cheers. Would you upload the screenshots as attachments to Trac itself? This way we won't lose them if CloudUp ever goes away. One of the screenshots for Nexus 6 failed to upload to CloudUp.

Sure, I uploaded the images to Trac. And I updated the URLs in the 2 comments I made.

#16 @westonruter
5 months ago

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

#17 @westonruter
5 months ago

  • Keywords needs-user-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

@ryankienstra I missed the comment from @celloexpressions:

I found a bug that I think is caused by this ticket. The Pages section now collapses when loading if you open it before it's loaded the initial items, and it also collapses whenever an infinite-scroll load happens.

Could you look at that?

#18 @ryankienstra
5 months ago

Bug In Expanding "Pages"

Hi @westonruter,
Sure, I'll look at the bug that @celloexpressions found. This is probably because the logic that opens the "Pages" section triggers a .click()event:
availableMenuItemContainer.find( '.accordion-section-title > button' ).click();

Normally, this would use a Customizer model, and call .expand(). But I couldn't find a model of this .accordion-section..

#19 @celloexpressions
5 months ago

Right, so maybe only call the click even if it's not already open?

@ryankienstra
5 months ago

#20 @ryankienstra
5 months ago

Pull Request And Identical Patch To Address Bug

Hi @westonruter and @celloexpressions,
This GitHub pull request and identical patch address the bug that @celloexpressions found.

In adding new menu items, sometimes the 'Pages' section collapsed

This is because the logic triggered .click(), even when it was open.

As @celloexpressions suggested, this commit checks if the section hasClass( 'open' ). And only triggers a click if it doesn't have that class.

#21 @celloexpressions
5 months ago

  • Keywords has-patch added; needs-patch removed

#22 @westonruter
5 months ago

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

In 38807:

Customize: Skip triggering initial click on pages section for available nav menu items if already open.

Fixes race condition if user opens Pages section before the ajax request to load items finishes.

Props ryankienstra, celloexpressions.
Fixes #36984.

Note: See TracTickets for help on using tickets.