#23119 closed enhancement (fixed)
UX Improvements to nav-menus.php
Reported by: | lessbloat | Owned by: | |
---|---|---|---|
Milestone: | 3.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Menus | Keywords: | 3.6-menus needs-codex |
Focuses: | Cc: |
Description (last modified by )
There are some UI issues with nav-menus.php that have been around for a while (like tabs being confusing for managing multiple menus).
Knowing this, we ran a couple of menus-based user tests hoping to find additional ideas for improvements.
Here's a list of potential enhancements that I'd like to test for 3.6:
A) Managing menus as tabs can be confusing. Sometimes users think the tabs represent the menu itself, and they'll create multiple menus by mistake (thinking the tabs represent menu items). Tabs also pose a problem, after you get 10+ menus. I'd like to test the idea of removing the tabs altogether. My thought was to add a "add menu" link next to the header (similar to other areas of the admin), and move menu selection to a drop down select box.
B) In the "add new menu" view, new users get confused as to what they are supposed to do. Both of the users we tested wanted to click the "Create Menu" button first (not noticing the menu name field). I'd like to test adding focus to the menu name field, and moving the "Create Menu" button closer to the name field (just for this add new screen).
C) The "theme locations" section appears to be semi-confusing to users. I'd like to test moving the functionality to the bottom of the page, and changing the formatting, so that it no longer matches the formatting of the rest of the left column, and perhaps changing the wording a bit.
D) I'd like to add an intro sentence which explains that menus can be used as custom navigation within your theme (we'd link to "theme locations" at the bottom of the page), or within widgets (adding a link to widgets). It will just be a quick, short sentence to introduce the concept of menus, while at the same time exposing the fact that menus can be used in widgets.
E) There's a lot of wasted space in the left column. Moving "theme locations" to the bottom of the page will free up a little space, but I'd like to see if there is anything else we can do to eliminate the need to scroll. I'd like to test changing the left column to an accordion (using jQuery UI). Thus eliminating a bunch of padding, and showing only one menu items options at a time. We'll test it and see how it performs.
F) When a user saves a change to "theme locations" it would be nice to give them a link to preview their new menu.
Once we complete some patches, we'll test these enhancements by re-running more users through the same set of user tests.
Attachments (53)
Change History (296)
#4
@
12 years ago
23119-C.diff includes B and:
- Removes the "Theme Locations" meta box
- Moves the "Theme Locations" functionality to the bottom of the page
- Reformats the "Theme Locations" section so that it no longer looks like the rest of the left hand menu options.
- Reworded "Theme Locations" as "Select menus for your theme" (copy suggestions welcome :-)).
Looks like:
#5
@
12 years ago
- Cc mdhansen@… added
I like the idea of using a select field in place of the text input and have an option to "add new" which then shows the text input with the create menu and a cancel button(to switch back to the select). I can create an example if anyone else is interested in this approach.
#9
@
12 years ago
23119-E.diff includes D, and turns the existing meta boxes into an accordion. I hacked it together with a few lines of JS and CSS. I chose to do it this way rather than mess with jQuery accordion, because all I'm really concerned with at this point is testing the concept. We'll test this version with some additional users and see how it performs (then we can come back and review the best way to handle this).
Looks like:
#10
@
12 years ago
23119-F.diff includes E, and adds a notification message when you select a menu for your theme that includes a link to preview your changes.
Looks like:
I'll run a couple users through these changes Monday, and post the results to the Make UI P2.
Feedback (and code reviews) welcome and very much appreciated.
#11
@
12 years ago
- Cc info@… added
Some nice ideas! I like it.
Setting the focus on load into the input field looks like a useful addition. But what happens when the user is just exploring the UI, looking for something else? It is quite a long way to get back to the left hand navigation without a mouse.
The new menu input could be a datalist
, so we can select a menu to edit without going down the whole page.
Oh, and I think Pages should be the top box on the left hand.
#12
@
12 years ago
23119.6.diff adds:
- localization functions
- blank-slate view when user has no menus
#14
@
12 years ago
This morning, I ran two more users through the same set of user tests as the first two users (from a week ago). From the above patches, B was the only only one that stood out as a clear improvement to me. I've reverted A, C, D, E & F in 23119.7.diff.
Next, I'll post another series of diffs based on observations from all 4 user tests, and then I'll test them again by running two more users through. We'll keep at it until we've smoothed over all of the rough parts.
#17
in reply to:
↑ 16
;
follow-up:
↓ 19
@
12 years ago
Replying to sooskriszta:
Please keep multilingual installations in mind too while designing new UI.
Since multilingual support is enabled via any number of plugins, and not built into core itself, I'm not sure how much if anything we can do to anticipate what plugins might or might not do to that screen.
#18
@
12 years ago
- Cc travis@… added
I think this all looks great! Very nice. My only (very, very minor) quibble with C is that I think the theme location settings should only be moved to the bottom if E is also done, so that the theme location settings aren't pushed way down on the page.
#19
in reply to:
↑ 17
@
12 years ago
Replying to helen:
Replying to sooskriszta:
Please keep multilingual installations in mind too while designing new UI.
any number of plugins...not sure ... anything we can do to anticipate what plugins might ...do to that screen.
I guess what I am asking is for you to look at the current behavior of the most popular plugin (WPML) or two (WPML, qTranslate) and extrapolate if possible.
Also I assume there might be more details about the under-development core functionality http://wordpress.org/extend/ideas/topic/multiple-languages-blogging-a-core-solution available to you than are to us.
Anyway, I don't want to hijack this discussion and if this is not relevant then I apologize.
#22
follow-up:
↓ 34
@
12 years ago
23119.8.diff focuses on improving copy, including:
Simplifying copy on the initial (and add new) screen from three sentences to one:
Changing "Theme Locations" to "Menus within your theme", and simplifying the copy in that meta box
Adding a line about dragging, and more info after items have been added to a menu:
Thoughts?
#23
follow-up:
↓ 25
@
12 years ago
The shorter initial description looks very good.
I think the line Automatically add … makes too much noise at its current position. Maybe set it below the input field and align both? Not sure how that works with translations.
Please, please move the most used metabox Pages to top.
#24
@
12 years ago
23119.9.diff adds a simple fade in effect to items when they are added to a menu (one user didn't notice where the menu item went after he saved it). It also adds a slideDown effect to the yellow message bar (just something I'd like to test).
#25
in reply to:
↑ 23
;
follow-up:
↓ 29
@
12 years ago
Replying to toscho:
Please, please move the most used metabox Pages to top.
I'm not totally against it. My only reservation is that by swapping the two, you no longer see both meta boxes at smaller resolutions. Currently you see "pages" peeking out at the bottom (making it more discoverable):
But if you swap the two, you only see:
#26
@
12 years ago
23119.10.diff adds a "sub menu" description to menu items that are dragged to any sub menu position. A couple of the users didn't really understand why the menu items could be dragged there. Hopefully this will provide a bit more clarity:
#27
follow-up:
↓ 28
@
12 years ago
Not sure "sub menu" is really quite right - I think it implies a display style, which might or might not be true in a given use. The concept of hierarchy is a little more of a complex one, but the help text should probably indicate "order and hierarchy" or some such.
#28
in reply to:
↑ 27
;
follow-up:
↓ 30
@
12 years ago
Replying to helen:
Not sure "sub menu" is really quite right - I think it implies a display style, which might or might not be true in a given use. The concept of hierarchy is a little more of a complex one, but the help text should probably indicate "order and hierarchy" or some such.
I see what you mean. How about "sub item", or "sub menu item"?
#29
in reply to:
↑ 25
@
12 years ago
Replying to lessbloat:
Replying to toscho:
Please, please move the most used metabox Pages to top.
I'm not totally against it. My only reservation is that by swapping the two, you no longer see both meta boxes at smaller resolutions. Currently you see "pages" peeking out at the bottom (making it more discoverable):
I think it makes sense to keep Pages second so you can see multiple boxes.
If you then minimise the custom box it should remember that so Pages will move higher.
#30
in reply to:
↑ 28
;
follow-up:
↓ 31
@
12 years ago
Replying to lessbloat:
I see what you mean. How about "sub item", or "sub menu item"?
"sub item" is certainly more accurate, although I'm going to play devil's advocate and wonder if we're wandering too far into trying to teach the concept of web navigation as a bulleted list with hierarchy via text on the menu screen :)
#31
in reply to:
↑ 30
@
12 years ago
Replying to helen:
"sub item" is certainly more accurate, although I'm going to play devil's advocate and wonder if we're wandering too far into trying to teach the concept of web navigation as a bulleted list with hierarchy via text on the menu screen :)
I'll try it out on a couple of users and see if they even notice it.
#32
@
12 years ago
Huge +1 to "Menus within your theme" or similar language. I think that'll be much easier to grok for most people.
#33
follow-up:
↓ 39
@
12 years ago
I believe that removing the tabs for listing all menus is necessary here.
What about a "New Menu" screen that has the most basic needs for a menu? Name, theme location, etc. Lots of room for help text. Creating the menu takes you to the menu manager page. This is more in line with a completely new menu than a submenu, though, and I doubt core devs would want to break it out of the Appearances menu.
If not that, an alternative is to use a Manage Menu/New Menu nav tab exactly like the Themes menu.
Removing the noise from what's needed to "build an existing menu" vs. "create a new menu" is what I'm getting at.
#34
in reply to:
↑ 22
;
follow-ups:
↓ 35
↓ 48
@
12 years ago
- Cc sararcannon@… added
The dragging/ordering verbiage takes up quite a bit of space for seasoned users (a whole menu item can practically fit there) - maybe we can intro-tooltip some of the menu directions for first time users?
also: any thoughts on collapsable submenus?
Replying to lessbloat:
23119.8.diff focuses on improving copy, including:
Adding a line about dragging, and more info after items have been added to a menu:
Thoughts?
#35
in reply to:
↑ 34
@
12 years ago
Replying to saracannon:
The dragging/ordering verbiage takes up quite a bit of space for seasoned users (a whole menu item can practically fit there) - maybe we can intro-tooltip some of the menu directions for first time users?
Or maybe that line can appear below the menu items?
If you add lots of menu items it will obviously be off the page but people should have seen it before that as they start adding items - unless it's a first time user editing an large existing menu I guess.
#39
in reply to:
↑ 33
;
follow-ups:
↓ 40
↓ 42
@
12 years ago
Replying to mmuro:
I believe that removing the tabs for listing all menus is necessary here.
What about a "New Menu" screen that has the most basic needs for a menu? Name, theme location, etc. Lots of room for help text. Creating the menu takes you to the menu manager page.
Riffing on this. What about something like:
Using WP_List_Table to list the menus instead of tabs.
Thoughts?
#40
in reply to:
↑ 39
@
12 years ago
Replying to lessbloat:
Using WP_List_Table to list the menus instead of tabs.
Oh, I like it. Easy to learn and to extend. And useful if need many menus, like in the case of one menu per language as mentioned earlier.
#42
in reply to:
↑ 39
;
follow-up:
↓ 44
@
12 years ago
Replying to lessbloat:
Replying to mmuro:
I believe that removing the tabs for listing all menus is necessary here.
What about a "New Menu" screen that has the most basic needs for a menu? Name, theme location, etc. Lots of room for help text. Creating the menu takes you to the menu manager page.
Riffing on this. What about something like:
Using WP_List_Table to list the menus instead of tabs.
Thoughts?
That's a good start. The theme location/menu assignment underneath the WP_List_Table class seems out of place, though. Would this make better sense as maybe a third column on the menu screen? Plus, that'll make it a sortable to help those with lots of meta boxes.
#43
@
12 years ago
That's a good start. The theme location/menu assignment underneath the WP_List_Table class seems out of place, though. Would this make better sense as maybe a third column on the menu screen? Plus, that'll make it a sortable to help those with lots of meta boxes.
Not sure how well that would work since you can only assign a single menu to a location, it might make for a really awkward UI to have that assignment be in the list of menus
#44
in reply to:
↑ 42
;
follow-up:
↓ 45
@
12 years ago
Replying to mmuro:
The theme location/menu assignment underneath the WP_List_Table class seems out of place, though. Would this make better sense as maybe a third column on the menu screen? Plus, that'll make it a sortable to help those with lots of meta boxes.
I don't think so. The theme location drop down allows you to select from multiple menus, which tells me it belongs on the menu management screen, not the add/edit screen (which should be all about a single menu). It always felt out of place to me in at the top of the add menu items column.
#45
in reply to:
↑ 44
@
12 years ago
Replying to lessbloat:
I don't think so. The theme location drop down allows you to select from multiple menus, which tells me it belongs on the menu management screen, not the add/edit screen (which should be all about a single menu). It always felt out of place to me in at the top of the add menu items column.
That's fair. I think you can make the menu mangement screen two column with the menu assignment on the left. With the WP_List_Class and it's paging/screen options, it could be pushed way down the page.
#47
@
12 years ago
I like the idea of using WP_List_Table for the menus and even still having the theme location metabox adjacent to it.
Has anyone considered maybe capitalizing on the post edit UI for menu editing as we similarly did for media editing in 3.5? Title becomes menu name, the metaboxes column could hold the "add menu item" metaboxes, etc. Just a thought.
#48
in reply to:
↑ 34
@
12 years ago
Technically speaking, this is a little bit different than attachments - a menu is a taxonomy term, and each nav menu item is a post in that post type associated with that taxonomy term (both the taxonomy and post type being non-exposed/non-public). Not saying that it's not an idea worth pursuing - just noting that it'd be more involved than what was done in 3.5 for the attachment post type.
Replying to saracannon:
also: any thoughts on collapsable submenus?
Total wish list item: collapsible and draggable branches. No idea about the feasibility or difficulty, so just a wish for now :)
#50
@
12 years ago
Replying to helen:
Technically speaking, this is a little bit different than attachments - a menu is a taxonomy term, and each nav menu item is a post in that post type associated with that taxonomy term (both the taxonomy and post type being non-exposed/non-public).
Oh, it's definitely a little bit different :)
The main reasoning I bring up a split UI is twofold:
- The UI as-is is cluttered and doesn't have a clear primary purpose. Currently, it's used for creating and managing menus as well as assigning menu locations. It's a vacuum-packed approach that is both difficult to organize and emphasize priority.
- A split UI allows for prioritizing the screens' purpose. A screen for managing menus and locations, a screen for editing the menus. The latter screen is familiar to people because it's re-used all over the Dashboard.
#52
@
12 years ago
It occurs to me that a split-UI approach should not be outside the realm of possibility but two unique pages should; that's another menu item under Appearance.
What about a stepped approach similar to how the WP-Table Reloaded plugin handles it? A management screen leads to a table-editing screen and you're given two buttons: "Update Changes" and "Save and go back".
So in this context you could have "Save Menu" and "Manage Menus" on the menu-editing step.
#54
@
12 years ago
23119.11.diff is a quick and dirty prototype of the concept in comment:39. I'll run a few users through this approach, and post results to the Make UI P2.
At a glance it looks like this:
Again, this is not committable code, just a quick proof of concept.
#55
follow-ups:
↓ 57
↓ 58
↓ 59
@
12 years ago
Would there be any support for breaking Menus out of the Appearance section? I've often thought that would make sense, seeing as how menus are essentially pieces of content, much like posts, pages, comments, other post types, etc. That would support using a UI similar to those other pieces of content (All Menus, Add New).
#56
@
12 years ago
Here are some notes for two new users testing the changes proposed in comment:54.
Overall, I think this looks like a good direction. I'd love to hear your thoughts.
#57
in reply to:
↑ 55
@
12 years ago
Replying to travisnorthcutt:
Would there be any support for breaking Menus out of the Appearance section? I've often thought that would make sense, seeing as how menus are essentially pieces of content, much like posts, pages, comments, other post types, etc. That would support using a UI similar to those other pieces of content (All Menus, Add New).
I was literally just thinking this. I know it would add to the 'noise' on the left-hand menu, but it would also fix the awkward tabs for manage/new.
#58
in reply to:
↑ 55
@
12 years ago
Replying to travisnorthcutt:
Would there be any support for breaking Menus out of the Appearance section? I've often thought that would make sense, seeing as how menus are essentially pieces of content, much like posts, pages, comments, other post types, etc. That would support using a UI similar to those other pieces of content (All Menus, Add New).
+1 for moving Menus out of Appearance. With a split UI it just makes more sense, avoids the problem users have with tabs on the Menu configuration page, works with the existing UI conventions for the Dashboard, etc.
Plus, Menus has always felt *buried* in there; most new users are unaware of this tool until I point it out, so I'm sure this is a pain point for a lot of new users.
#59
in reply to:
↑ 55
;
follow-ups:
↓ 60
↓ 61
@
12 years ago
Replying to travisnorthcutt:
Would there be any support for breaking Menus out of the Appearance section?
My worry with this would be, how often do you customize menus? My guess is that many (dare I say most) users will do it once to add menus while first customizing their site, and then only very occasionally revisit to make small changes. So, while conceptually it make sense (since there is both a management screen, and an add/edit screen), I'm still not certain it warrants a spot in the main nav.
Thoughts?
#60
in reply to:
↑ 59
;
follow-up:
↓ 62
@
12 years ago
I agree with @lessbloat on leaving it as-is under Appearance. We can have two screens (two files, whatever) without needing two menu items. And as @helen points out, menus aren't your fly-by-night average post type, in fact they are a taxonomy.
I would however still argue for 2 separate files (the same as how we handle the two tabs in Appearance > Themes), but that's mostly because it's easier to do targeted help tabs without having to stuff them full of two screens' information.
#61
in reply to:
↑ 59
@
12 years ago
Replying to lessbloat:
My worry with this would be, how often do you customize menus? My guess is that many (dare I say most) users will do it once to add menus while first customizing their site, and then only very occasionally revisit to make small changes. So, while conceptually it make sense (since there is both a management screen, and an add/edit screen), I'm still not certain it warrants a spot in the main nav.
Thoughts?
That's a fair argument; I don't know any users that change this once it's been set up, except maybe occasionally.
#62
in reply to:
↑ 60
@
12 years ago
Replying to DrewAPicture:
I would however still argue for 2 separate files (the same as how we handle the two tabs in Appearance > Themes), but that's mostly because it's easier to do targeted help tabs without having to stuff them full of two screens' information.
Sounds fine by me. The converse however is that you then have two pages with very similar markup. Anyone else want to weigh in on this?
#63
@
12 years ago
Extending the WP_List_Class will require it's own file anyways. I think the switching of tabs can be done with one file.
#66
@
12 years ago
23119.12.diff takes the user back to the "add" screen if they delete a menu, and there are no other menus vs. just showing them an empty manage menus table.
#67
@
12 years ago
Also, as a heads up @jkudish is working on transitioning my hacky menu management table (in 23119.11.diff) to use WP_List_Table.
#69
@
12 years ago
23119.14.diff:
- Adds a new "Common Links" meta box with "home", and "log in" for starters. This is hacked in (I'm just anxious to test it), so if you can think of a better way to add it (perhaps make it more extensible) I'm all ears.
- Re-organizes the meta boxes on the "add/edit" screen, so now it's: (Pages, Common Links, Custom Links, Categories) by default.
- Changed "Label" under "Custom Links" to "Link Text". We'll test this and see how it performs.
Here's what it looks like:
Thoughts?
#70
follow-ups:
↓ 73
↓ 74
@
12 years ago
- Keywords has-patch needs-testing added
23119.15.diff builds upon 23119.14.diff and converts the "hacked up" table into a proper WP_List_Table
, which gives it several features:
- search
- items per page + paging
- bulk actions
- cleaner code to maintain / easier extendability
Here's how it looks now. Note that I chose that location for the search bar because I thought it would be a better spot for it vs. above the table. Up for debate of course.
Implementation notes:
I pretty much went with an approach of the least modified code possible to integrate the WP_List_Table
. There's definitely more work to be done:
- I suggest we split the two tabs into 2 separate files, this will allow us to cut down on the amount of logic/if statements to display the different UI elements and will make updates easier
- There's cleanup to be done with some (now) old code, assuming core adopts this new UI.
Thoughts so far?
#71
follow-up:
↓ 96
@
12 years ago
@lessbloat: some thoughts on 23119.14.diff:
What other links do you imagine under "Common Links"? Do you expect this to auto-populate with actual frequently-used links as users add items to their menus? Or is it just meant to be a specially-curated list of links?
If the latter, then I suggest that we rename it to something different, as "common" suggests to me that they would be links I've inserted before. Also, wonder if anyone has extra suggestions as for links to add in there? Seems silly to have that meta box for just 2 links. Then again we should make it extensible for plugin developers to add their own links in here.
#72
@
12 years ago
I realized that deleting doesn't work (yet) neither from the action button (on hover) nor as a bulk action. I will fix it later today and submit an updated patch.
#73
in reply to:
↑ 70
@
12 years ago
Replying to jkudish:
Note that I chose that location for the search bar because I thought it would be a better spot for it vs. above the table. Up for debate of course.
I guess above the table would be more intuitive and consistent with the other list tables.
#74
in reply to:
↑ 70
@
12 years ago
Replying to jkudish:
I like the added search bar on top of the menu location meta box, but now that it's there, I wish both of them were on the other side of the list table, e.g. 2col:1col vs 1col:2col.
The search box seems confusing to me when visually paired with the metabox because the list table is what's going to change if you search for something. And actually, what would the workflow be if you searched? Does it just filter down the list table view?
I haven't poked into 23119.14 yet, so just wondering about row actions. Has there been discussion about what row actions we might want for the list view?
#75
follow-up:
↓ 76
@
12 years ago
Replying to SergeyBiryukov:
And actually, what would the workflow be if you searched? Does it just filter down the list table view?
Yes, that's correct, it uses the s
paramater from wp_get_nav_menu_items
which trickles down to get_terms
and performs a %search_term%
LIKE
query.
Reply to DrewAPicture
I haven't poked into 23119.14 yet, so just wondering about row actions. Has there been discussion about what row actions we might want for the list view?
For now the only row actions are 'Delete' and 'Edit'. lessbloat suggested adding a duplicate function, but that should be a separate ticket and would involve extra API work to allow duplicating menus.
In 23119.16.diff:
- Deletion and bulk deletion now works.
- Search now only appears if there are more than X number of menus, where X is currently set to 20 (which is the default
per_page
value). This value is filterable by plugins of course. - Search bar (when visible) now appears above the table, for consistency's sake.
- Some code-cleanup but there's still lots of that to do.
#76
in reply to:
↑ 75
@
12 years ago
Replying to jkudish:
In 23119.16.diff:
- Deletion and bulk deletion now works.
- Search now only appears if there are more than X number of menus, where X is currently set to 20 (which is the default
per_page
value). This value is filterable by plugins of course.- Search bar (when visible) now appears above the table, for consistency's sake.
- Some code-cleanup but there's still lots of that to do.
Awesome stuff jkudish! I'll kick off another round of user tests tomorrow.
#77
follow-ups:
↓ 78
↓ 79
↓ 85
↓ 95
@
12 years ago
Interested to see all the debate about the UI for the Menus area.
Would please ask that we also consider accessibility in the discussion. There is an outstanding trac ticket #14045 (and duplicate #21289) about accessibility of the custom menu builder. Basically it is impossible or very difficult to be able to fully create a custom menu and amend the hierarchy if you can't use a mouse. The drag and drop functionality is an alien concept to blind users who will probably be using keyboard interaction with their screen readers.
Any solution to the Menus functionality should either be a) accessible for everyone, or b) an alternate accessible version should be provided alongside the drag/drop interface (as per Widgets).
Have heard mention on another ticket that there might be a rudimentary accessible non-js version in place already. Is that actually true, and could it form the basis for an accessible way forward?
#78
in reply to:
↑ 77
@
12 years ago
Replying to grahamarmfield:
Interested to see all the debate about the UI for the Menus area.
Would please ask that we also consider accessibility in the discussion.
Hi Graham,
I agree. This is definitely on my list. I will do my best to address this in 3.6. :-)
#79
in reply to:
↑ 77
@
12 years ago
Replying to grahamarmfield:
Have heard mention on another ticket that there might be a rudimentary accessible non-js version in place already. Is that actually true, and could it form the basis for an accessible way forward?
With JavaScript turned off, there are up/down links that appear to change the order of each item.
#80
@
12 years ago
A couple of minor CSS tweaks to the add new screen after you've deleted all of your menus in 23119.17.diff.
#81
@
12 years ago
Hope I'm not too late to the party.
I Believe the double-tab separation between Management and Creation modes is really a nice and useful addition; should make all the difference for users to make the distinction naturally over time.
I only have one suggestion:
Add Menu
I feel like the menu name hasn't enough prominence where it is right now. I'd try to put it outside the box to make the information hierarchy clearer (outside defines how the menu is going to be defined; inside defines what belongs to this context).
I really like where this is headed!
#83
@
12 years ago
Results for user test 7 & 8 are in. We've made some great progress here.
#84
@
12 years ago
Here are the items that I have left on my list:
- Break this out into two files 1) manage 2) add/edit
- Look into http://core.trac.wordpress.org/ticket/16075 (@DrewAPicture volunteered to look into this)
- Look into potential of adding JS tree collapsing functionality to menu items
- Look into way for theme devs to easily extend new "Common Links" meta box
- Updated copy for help tabs (both manage, and add/edit screen) (@DrewAPicture will oversee this)
- Updated copy for http://codex.wordpress.org/WordPress_Menu_User_Guide (@DrewAPicture will oversee this)
- Updated copy for http://codex.wordpress.org/Appearance_Menus_Screen (@DrewAPicture will oversee this)
- Accessibility (keyboard reordering) (I'll work on this one - @lessbloat)
- Find someone to code review (and fix any of my hacky code) :-)
- RTL testing
- Browser compatibility testing
- No JS testing
Did I miss anything? If you see something you'd like to tackle, please speak up. :-)
#85
in reply to:
↑ 77
@
12 years ago
Replying to grahamarmfield:
23119.18.diff is a start, adding "up" and "down" arrow keyboard accessibility. This allows you to reorganize items vertically within a menu. You just:
- Tab to a menu item
- press the up or down arrow key (the menu item will move up or down accordingly)
- "Save your menu changes.
I'll try and work on "left" and "right" next week.
#86
follow-up:
↓ 87
@
12 years ago
From user test 7, I posted a comment about rewording the "menu updated" message links.
I also suggested adding a "Manage your widget menus" link to the theme location metabox.
Last point, it would be nice to list the theme location as a column in the list table even though we have the metabox adjacent to the list. It just seems like something that you'd expect to be listed.
#87
in reply to:
↑ 86
;
follow-up:
↓ 90
@
12 years ago
Replying to DrewAPicture:
From user test 7, I posted a comment about rewording the "menu updated" message links.
I like it.
I also suggested adding a "Manage your widget menus" link to the theme location metabox.
Can you show us what you had in mind by mocking this up?
Last point, it would be nice to list the theme location as a column in the list table even though we have the metabox adjacent to the list. It just seems like something that you'd expect to be listed.
Good idea. I agree.
#89
follow-up:
↓ 91
@
12 years ago
Is "Use this menu within your theme" locked in for the metabox? If so, I'd almost be tempted to change "Assign a theme location for this menu" to mimic that. As I mentioned on Skype, it might also be worth it to swap out the anchor text based on whether the menu already has an assignment -- something I'll play with.
#90
in reply to:
↑ 87
;
follow-up:
↓ 92
@
12 years ago
Replying to lessbloat:
Replying to DrewAPicture:
I also suggested adding a "Manage your widget menus" link to the theme location metabox.
Can you show us what you had in mind by mocking this up?
I was thinking something along these lines:
#91
in reply to:
↑ 89
;
follow-up:
↓ 93
@
12 years ago
Replying to DrewAPicture:
Is "Use this menu within your theme" locked in for the metabox?
Nope. What did you have in mind?
#92
in reply to:
↑ 90
;
follow-up:
↓ 94
@
12 years ago
Replying to DrewAPicture:
I was thinking something along these lines:
Linking to widgets is something I've not fully worked out yet. I like that we're exposing the fact that you can use menus in widgets. My only worry is what happens when the user clicks the link? There is a lot on that page, and I think it will be hard for most new users to make the connection that they then need to 1) add a "custom menu" widget to their sidebar, and then 2) select a menu within that widget and save it. Wondering if there is anything else we can do there to bring more clarity to this process.
#93
in reply to:
↑ 91
@
12 years ago
Replying to lessbloat:
Replying to DrewAPicture:
Is "Use this menu within your theme" locked in for the metabox?
Nope. What did you have in mind?
I like it, probably better than the 'Assign a theme location for this menu' anchor on the update message. Consistency in messaging is better because people read it, click the link, repeat it and look for something that matches. And if it matches exactly, all the better.
#94
in reply to:
↑ 92
@
12 years ago
Replying to lessbloat:
Linking to widgets is something I've not fully worked out yet. I like that we're exposing the fact that you can use menus in widgets. My only worry is what happens when the user clicks the link? There is a lot on that page, and I think it will be hard for most new users to make the connection that they then need to 1) add a "custom menu" widget to their sidebar, and then 2) select a menu within that widget and save it. Wondering if there is anything else we can do there to bring more clarity to this process.
For sure the last thing we want to do is dump a user on the Widgets page to connect the dots for themselves. How about just some hint text, like:
"You can also use the Custom Menu Widget to add menus to the sidebar of your site. Manage my widgets."
That's a bit wordy, but at least points the user in the right direction...
#95
in reply to:
↑ 77
;
follow-up:
↓ 179
@
12 years ago
Replying to grahamarmfield:
23119.19.diff adds full keyboard accessibility for re-arranging menu items.
Testing
1) Focus on a menu item (there has to be more than one)
2) pressing the right, left, up or down key.
Supports RTL. Supports moving single item as well as items with children (sub items). Ideally, it should behave identical to drag/drop. With that said, there is a lot going on here, so please ping me if you notice any issues.
Patch also adds class-wp-menus-list-table.php back in (was missing in 23119.18.1.diff)
#96
in reply to:
↑ 71
;
follow-ups:
↓ 98
↓ 99
@
12 years ago
Replying to jkudish:
@lessbloat: some thoughts on 23119.14.diff:
What other links do you imagine under "Common Links"? Do you expect this to auto-populate with actual frequently-used links as users add items to their menus? Or is it just meant to be a specially-curated list of links?
My thought was to show "Home" and "login" by default. Mostly because these are particularly hard to figure out how to add to the menu. But then to allow theme devs to hook in and add/remove whatever links they want. I don't think we should auto-populate with frequently-used links.
If the latter, then I suggest that we rename it to something different, as "common" suggests to me that they would be links I've inserted before.
Open to suggestions. How about "Helpful links", or "Quick links"?
Also, wonder if anyone has extra suggestions as for links to add in there? Seems silly to have that meta box for just 2 links.
I think it's okay to have just 2 links. I don't want to add too many, or we'll lose the "Custom Links" meta box at the bottom fold.
#98
in reply to:
↑ 96
@
12 years ago
Replying to lessbloat:
Replying to jkudish:
Also, wonder if anyone has extra suggestions as for links to add in there? Seems silly to have that meta box for just 2 links.
I think it's okay to have just 2 links. I don't want to add too many, or we'll lose the "Custom Links" meta box at the bottom fold.
How about the latest post from all post types available in nav menus?
So when I have written a new page I can head over to menu admin and will find that page immediately.
#99
in reply to:
↑ 96
;
follow-up:
↓ 100
@
12 years ago
23119.20.diff adds a filter for the $common_links
array, also swaps out get_site_url()
for get_home_url()
on the default 'Home' link.
Just a note about "Home" links. As long as we keep "common links" as the type "custom" (vs page, post, CPT, taxonomy, etc.), there's code already in place to make sure the home links can be recognized as the "current" page. See wping/nav-menu-template.php for more on that.
Replying to lessbloat:
Replying to jkudish:
If the latter, then I suggest that we rename it to something different, as "common" suggests to me that they would be links I've inserted before.
Open to suggestions. How about "Helpful links", or "Quick links"?
What about "Useful Links"?
#100
in reply to:
↑ 99
@
12 years ago
Replying to DrewAPicture:
Replying to lessbloat:
Replying to jkudish:
If the latter, then I suggest that we rename it to something different, as "common" suggests to me that they would be links I've inserted before.
Open to suggestions. How about "Helpful links", or "Quick links"?
What about "Useful Links"?
I like "Useful Links" and "Quick Links" the most out of all the suggestions so far.
#101
follow-up:
↓ 102
@
12 years ago
Is it necessarily good to have a common links/useful links/quick links/etc. box? I'm not necessarily opposed, but I think it's worth examining before adding that.
#102
in reply to:
↑ 101
;
follow-up:
↓ 109
@
12 years ago
Replying to travisnorthcutt:
Is it necessarily good to have a common links/useful links/quick links/etc. box? I'm not necessarily opposed, but I think it's worth examining before adding that.
I think it's a nice-to-have, and could be a nice way of tying up some loose ends:
- Moving the 'Home' link out of the Pages metabox #14325
- Make it easy to add a link to the login form (which many users have trouble with) comment:96
- Provide a filter for plugins to specify additional useful links via a filter comment:99
I especially see the benefit for plugins that may have specialty feeds or pages to link to. As it stands, if you want anything custom, you have to type it out yourself.
#106
@
12 years ago
- Summary changed from UI Improvements to nav-menus.php to UX Improvements to nav-menus.php
#107
@
12 years ago
From IRC:
lessbloat - jkudish: Do you think we could do away with bulk actions on the menus list screen? I just think it will be pretty edge case that someone has enough menus to need them, and if we got rid of them, the list table would align with the "Menus within your theme" meta box.
jkudish - yes can do. can you add that to the ticket so I remember?
And hopefully, it will likely make that screen slightly less heavy.
Here's what it will look like:
#108
follow-ups:
↓ 111
↓ 112
↓ 114
@
12 years ago
- Cc nashwan.doaqan@… added
I have not testing the new patches until now , but I want to talk about some difficulties I founded in the previous WordPress versions ..
1 - It's hard to move\delete more than one menu element in the same time .
2 - When Adding a page with it's child pages the users expect that the child pages be a child elements in the menu too ! .
#109
in reply to:
↑ 102
@
12 years ago
Replying to DrewAPicture:
I especially see the benefit for plugins that may have specialty feeds or pages to link to. As it stands, if you want anything custom, you have to type it out yourself.
Yes , I agree with you I was thinking about something like that before a few months , It's very good to plugins and themes to give the user more powerful menus .
Another Idea is to add a context menu items , I means add the ability to show items for the logged-in-users or shows items in home page only ... etc ... I think it's a plugin job but it still a good idea :)
#111
in reply to:
↑ 108
;
follow-up:
↓ 113
@
12 years ago
Replying to alex-ye:
2 - When Adding a page with it's child pages the users expect that the child pages be a child elements in the menu too ! .
I couldn't agree more with this. The ability to automatically add child pages (preferably with a checkbox that can be turned on or off depending on preference) would be a great improvement to menus.
Even though Menu's are separate from the actual page structure, in my experience many novice users assume that child pages will get added with the top level page automatically.
The best case senario would be to have all subpages physically added (when the option is checked) so that individual subpages can be removed if necessary.
#112
in reply to:
↑ 108
;
follow-up:
↓ 124
@
12 years ago
Replying to alex-ye:
2 - When Adding a page with it's child pages the users expect that the child pages be a child elements in the menu too ! .
FWIW, there's a plugin for that:
http://wordpress.org/extend/plugins/add-descendants-as-submenu-items/
#113
in reply to:
↑ 111
@
12 years ago
Replying to lachlanj:
I couldn't agree more with this. The ability to automatically add child pages (preferably with a checkbox that can be turned on or off depending on preference) would be a great improvement to menus.
Even though Menu's are separate from the actual page structure, in my experience many novice users assume that child pages will get added with the top level page automatically.
The best case senario would be to have all subpages physically added (when the option is checked) so that individual subpages can be removed if necessary.
I see how you could make a case for this, but to be fair, when you add pages to a custom menu, what you see is then an accurate representation of what the menu will be after saving, so it's not as if any behavior is hidden from the user.
Furthermore, the default behavior (which I just tested in 3.5 with the Twenty Eleven theme active), with no custom menus created, is to add all pages to the menu, including child pages, as sub-menu items under their parent pages. Because of this behavior (which is good, imo), adding a checkbox to the custom menu creation area titled something like "Add all child pages automatically" would be rather ambiguous, as it could have five meanings that I can think of right now, depending on where it was placed:
- Only add currently extant pages which have the page I'm actively adding as their parent. If new child pages are created later, do not add them automatically.
- Regardless of when they come into existence, add all child pages of the page I'm actively adding to this menu. If I add a new child page next week, add it to the menu.
- Do # 1, but for all parent pages currently in the menu.
- Do # 2, but for all parent pages currently in the menu.
- Do # 1, but for all parent pages, regardless of when they are added to the menu.
- Do # 2, but for all parent pages, regardless of when they are added to the menu.
I should note that this list went from two scenarios to six scenarios as I was typing it. Suffice it to say, I think adding such a checkbox has the potential to introduce a lot of confusion and complexity! However, like I said, I could see a case (whether I agree with it or not) for it, so perhaps it would be worth creating a ticket to discuss.
#114
in reply to:
↑ 108
;
follow-up:
↓ 123
@
12 years ago
Replying to alex-ye:
1 - It's hard to move\delete more than one menu element in the same time
Any thoughts on how we might remedy this?
2 - When Adding a page with it's child pages the users expect that the child pages be a child elements in the menu too ! .
My vote would be to keep this as a plugin (comment:112).
#116
follow-ups:
↓ 117
↓ 118
@
12 years ago
I've been thinking about the proposed new layout over the weekend, and the one thing that still bugs me is the "Menus within your theme" meta box. It still seems out of place. I mocked up an alternative approach, and wanted to get your take:
What if we removed the "Menus within your theme" meta box from the manage screen.
And put it at the top of "Screen Options":
Then the "default menu for your theme" link in the "Save Menu" success message would simply slide down "Screen Options" when clicked.
We of course would user test this approach to find out if it actually works for users.
Thoughts?
#117
in reply to:
↑ 116
@
12 years ago
Replying to lessbloat:
I've been thinking about the proposed new layout over the weekend, and the one thing that still bugs me is the "Menus within your theme" meta box. It still seems out of place. I mocked up an alternative approach, and wanted to get your take:
What if we removed the "Menus within your theme" meta box from the manage screen.
And put it at the top of "Screen Options":
Then the "default menu for your theme" link in the "Save Menu" success message would simply slide down "Screen Options" when clicked.
We of course would user test this approach to find out if it actually works for users.
Thoughts?
The problem with this approach is that you are hiding primary functionality within the Screen Options tab, which should be used to show/hide items in the admin screen.
If you want to get rid of that meta box, you might as well get rid of the two tabs and use the header "Add New" method and just display it on the menu screen in the same spot it's always been.
#118
in reply to:
↑ 116
;
follow-up:
↓ 119
@
12 years ago
Replying to lessbloat:
What if we removed the "Menus within your theme" meta box from the manage screen.
I prefer this approach. The "Menus within your theme" should be controlled from the "Theme Customizer".
#119
in reply to:
↑ 118
;
follow-up:
↓ 133
@
12 years ago
Replying to mmuro:
The problem with this approach is that you are hiding primary functionality within the Screen Options tab, which should be used to show/hide items in the admin screen.
This is true...
Well scratch that idea. ;-)
Replying to ramiy:
I prefer this approach. The "Menus within your theme" should be controlled from the "Theme Customizer".
I don't think that all themes have a "Theme Customizer" page, so I'm not sure that will work either. Plus this seems like it would be just as hard to discover.
#120
follow-up:
↓ 121
@
12 years ago
@lessbloat sounds like you've already decided this way, but +1 on not hiding the "menus in your theme" functionality like that.
#121
in reply to:
↑ 120
@
12 years ago
Replying to travisnorthcutt:
@lessbloat sounds like you've already decided this way, but +1 on not hiding the "menus in your theme" functionality like that.
Yep, but they still bug me as is... :-)
#122
@
12 years ago
+1 to not hiding in another area of appearance.
They should be managed somehow within the Menus section I think.
Another tab for "Theme Menus" perhaps?
Not sure about that, just throwing out another suggestion.
#123
in reply to:
↑ 114
@
12 years ago
Replying to lessbloat:
Any thoughts on how we might remedy this?
mmm , I am not sure about the best way to achieve this but I have an idea on how can we do it , Simply adding a check-box beside the menu item title , so by checking more than one item makes easy to move or delete ... etc
The image source from comment:81 :)
#124
in reply to:
↑ 112
;
follow-up:
↓ 127
@
12 years ago
Replying to SergeyBiryukov:
FWIW, there's a plugin for that:
http://wordpress.org/extend/plugins/add-descendants-as-submenu-items/
BOF , You know that many of WordPress core features comes from plugins or themes , Maybe it's not worth to you but it worth a lot for many users out there :)
#125
follow-up:
↓ 128
@
12 years ago
I also took some time over the weekend to sort of hone in on what exactly the problem is that we're solving. Having not watched the initial tests, I came to the conclusion that we're really trying to relieve two main pain points:
- Managing multiple menus in an easily discoverable way
- Cramming the workflow into a single screen
At the devchat last week, @nacin made a pretty valid point that the proposed two-tab workflow feels "heavy". I think that if we step back a little bit at look at both the pain points we're trying to address and the ideas we've come up with thus far, we can take it from a new, educated approach.
If you look at, for instance, the way @lessbloat was able to direct the focus on the "New Menu" screen by using an overlay to disable anything but the new menu creation form, that's something that works really well to promote visual hierarchy.
What if we do something like this:
Menus overall workflow:
- Return to the original idea of a single-screen menus workflow
Menu location metabox & multiple menu management:
- Move the idea of assigning a menu to a location into the menu editor itself, therefore tying it more to the workflow of, "I'm editing this menu, I want it to be used in 'this place'"
- Drop using the tabbed navigation system for switching between menus. In my experience, users seem confused by this most of the time.
- Leverage the "location" metabox/area to instead act as the primary launchpad for toggling between menus and use the overlay effect to highlight this area when you first hit the page.
I can do a mockup of this once I get to the airport but that's the gist of it. It's the equivalent of a UI refresh but not a complete overhaul.
#126
@
12 years ago
Replying to travisnorthcutt:
adding a checkbox to the custom menu creation area titled something like "Add all child pages automatically" would be rather ambiguous .
I agree with you , What about adding a checkbox in the Page Meta Box ( or any post-type that supports Hierarchical ) shows when selecting a parent item ( Item that have a child items ) titled by something like 'Hierarchical ?' ...
#127
in reply to:
↑ 124
@
12 years ago
Replying to alex-ye:
adding a check-box beside the menu item title , so by checking more than one item makes easy to move or delete ... etc
I worry about this for 2 reasons:
1) It will add complexity to an already cluttered UI
2) I'm not convinced that 80% of users will find this essential functionality.
For these reasons, I'd vote to keep "batch menu item actions" as a great idea for a plugin, but not a perfect fit for core.
Replying to alex-ye:
BOF , You know that many of WordPress core features comes from plugins or themes , Maybe it's not worth to you but it worth a lot for many users out there :)
I also feel like this is a great idea for a plugin, but overkill for core.
In the end, my goal is to have menus be clean, lean, and mean for the majority of users. But that doesn't mean that they can't be extended (through plugins) for additional use cases.
#128
in reply to:
↑ 125
@
12 years ago
Replying to DrewAPicture:
At the devchat last week, @nacin made a pretty valid point that the proposed two-tab workflow feels "heavy".
I'm not particularly attached to the idea of having a manage screen and an add/edit screen. With that said, I do expect any concept we consider to test as well, if not better than the 2 tab approach.
The only measure of effectiveness that I have to fall back on is that the two tab approach seemed to test well (especially since the users were only working with 1 or 2 menus). Neither of them seemed confused, or overwhelmed by the manage screen.
What if we do something like this:
Menus overall workflow:
- Return to the original idea of a single-screen menus workflow
Menu location metabox & multiple menu management:
- Move the idea of assigning a menu to a location into the menu editor itself, therefore tying it more to the workflow of, "I'm editing this menu, I want it to be used in 'this place'"
- Drop using the tabbed navigation system for switching between menus. In my experience, users seem confused by this most of the time.
- Leverage the "location" metabox/area to instead act as the primary launchpad for toggling between menus and use the overlay effect to highlight this area when you first hit the page.
I'm happy to test anything we think might perform better. But my gut says that any single screen that attempts to:
1) manage menus
2) add menus,
3) edit menus, and
4) manage theme locations
is going to end up being somewhat complex. But again, please mock something up, I'd be happy to test it.
#129
@
12 years ago
As a heads up I just ran one more user through to test the 2 tab concept. I was looking to test an idea I had about visually highlighting the “Menus in your theme” meta box when a user clicks there from the “add/edit” screen success message. It doesn't appear to have helped in that regard, but using the two tabs still seems to offer a fairly simple flow overall.
#130
@
12 years ago
I was travelling so I am just catching up on the conversation here and going to quickly throw in my thoughts on everything that's been said (but will bare repeating at the Menus discussion in a few minutes as well. Essentially I agree with @lessbloat on most of his points, but will elaborate:
- I agree in regards to complex functionality, that should stay in plugins and we should keep the UI minimal and clean while affording functionality for ~80% of users
- I am also not married to the two tab UI, but just as the test confirm, it's a much better interface than what we had before. If someone makes a good mockup that shows a redo of the one tabbed UI, I might jump ship :)
- The "locations" meta box was a bit odd on the right side. But it also most definitely does not belong hidden in "screen options", that's a really bad place for it. I think that either below or above the list of menus may be a better place for it. It's hard to only display it when editing a specific menu since it's the kind of thing that could be changed without needing edit a menu.
Just my 2 cents. Looking forward to the discussion shortly.
#132
@
12 years ago
Perhaps the menu name/save area has room to grow. This might be a good spot for assigning them to a theme and a little slide toggle could be used. I use this exact method in one of my plugins to allow for lots of different options that wouldn't otherwise fit.
#133
in reply to:
↑ 119
;
follow-up:
↓ 134
@
12 years ago
Guys, let's examine again the "Theme Customizer" (or "Live Preview") approach - to change the theme design workflow.
The new worklfow i propose:
- Manage menus/sidebars - both Menus UI and Widgets UI will use WP_List_Table to add/edit/delete custom menus and sidebars.
- Manage theme locations - to assign specific menu to a theme-location (or widget-set to theme-sidebar) we will do it throw the theme live preview.
Which means - remove the "Menus within your theme" meta box from the Menus screen.
Disadvantages:
- Two step workflow.
- New approach, hard to adopt.
Advantages:
- Increase "Live Preview" usage.
- Same workflow for menus and widgets.
- All the design changes will be controlled from one place ("Live Preview"), and the user will see how the changes effect his site.
#134
in reply to:
↑ 133
;
follow-up:
↓ 135
@
12 years ago
Replying to ramiy:
Guys, let's examine again the "Theme Customizer" (or "Live Preview") approach - to change the theme design workflow.
In theory this is a good place for picking the in-theme menu locations, and one we should accommodate/allow for. However since most or at least not all themes support the theme customizer, and we need to remain backwards compatible, we can't have this as the only way to set theme menu locations.
#135
in reply to:
↑ 134
;
follow-up:
↓ 136
@
12 years ago
Replying to jkudish:
Since most or at least not all themes support the theme customizer, and we need to remain backwards compatible, we can't have this as the only way to set theme menu locations.
We have backwards compatibility.
The "Theme Customizeris" was added to WordPress in 3.4, it's not a Theme-Feature that need activation, it's an admin screen.
#136
in reply to:
↑ 135
@
12 years ago
Replying to ramiy:
it's not a Theme-Feature that need activation, it's an admin screen.
For some reason I thought that a theme needed to add theme support for the customizer. I tested it out and you're right. My mistake :)
I noticed that menu theme locations are already available in the customizer though...
#137
follow-ups:
↓ 138
↓ 140
@
12 years ago
Keep in mind: themes aren't the only place menus can be used, so we need to be careful that we don't force a workflow that assumes they're only used in themes. I believe lachlanj has input on that and also some use cases to give a better sense of how menus are actually used in the wild/production.
#138
in reply to:
↑ 137
@
12 years ago
Replying to rmccue:
Keep in mind: themes aren't the only place menus can be used, so we need to be careful that we don't force a workflow that assumes they're only used in themes. I believe lachlanj has input on that and also some use cases to give a better sense of how menus are actually used in the wild/production.
I know that menus also used in widgets, it doesn't break anything. We just change the UI and present a new User Experience for changing theme design.
#139
@
12 years ago
It is possible to use a menu in wp-login.php
only, activated per plugin. The widget manager or the theme customizer don’t help in these cases.
#140
in reply to:
↑ 137
@
12 years ago
Replying to rmccue:
Keep in mind: themes aren't the only place menus can be used, so we need to be careful that we don't force a workflow that assumes they're only used in themes. I believe lachlanj has input on that and also some use cases to give a better sense of how menus are actually used in the wild/production.
Thanks rmccue, I thought it might be helpful to show 2 different 'real world' cases for how we have used menus on client sites.
- http://cl.ly/image/3i2v3C3H0I2v - This theme has 3 menus locations, while 2 other menus (Download Sidebar and Video Sidebar) are used in widget areas.
- http://cl.ly/image/0l3p2j1F3p2y - This theme uses 7 menus locations in total, 3 standard menus but the theme also has 5 different landing pages for each department in the company, and each of these has it's own unique menu.
This is similar to what has been discussed above, but I thought some screenshots might help
#141
follow-up:
↓ 142
@
12 years ago
This is what i was thinking:
When themes register menus and sidebars,
// Register Navigation Menus function custom_navigation_menus() { $locations = array( 'primary_menu' => __( 'Primary Menu', 'text_domain' ), ); register_nav_menus( $locations ); } add_action( 'init', 'custom_navigation_menus' ); // Register Sidebar function custom_sidebar() { $args = array( 'id' => 'unique_id', 'name' => __( 'Sidebars', 'text_domain' ), 'description' => __( 'Sidebar Description', 'text_domain' ), 'class' => '', 'before_title' => '<h2 class=\"widgettitle\">', 'after_title' => '</h2>', 'before_widget' => '<li id=\"%1$s\" class=\"widget %2$s\">', 'after_widget' => '</li>', ); register_sidebar( $args ); } add_action( 'widgets_init', 'custom_sidebar' ); // Code generated using GenerateWP.com
We will see both Menus and Sidebars on the "Theme Customizer".
The Menu screen and the Widget screen will be used to manage links & wigets.
The "Customizer" will be used to assign specific menu/sidebar to the theme.
This is basicly the Two step workflow.
#142
in reply to:
↑ 141
;
follow-up:
↓ 143
@
12 years ago
Replying to ramiy:
This is what i was thinking:
The "Customizer" will be used to assign specific menu/sidebar to the theme.
Please note, we are only focusing on menus improvements for this ticket (but I hope to see improvements to widgets in a future release as well).
Assigning the theme location inside the customizer is definitely worth testing (especially since it's already coded up). It would eliminate the meta box from the manage screen, and give the user a visual representation of the change (and how it affects their theme). If we continue to support IE7 in 3.6, we'd have to come up with a work around, since the customizer doesn't work (that I recall) in IE7.
#143
in reply to:
↑ 142
@
12 years ago
Replying to lessbloat:
If we continue to support IE7 in 3.6, we'd have to come up with a work around, since the customizer doesn't work (that I recall) in IE7.
WordPress 3.2 drop IE6 support and Start End-of-life (EOL) cycle for IE7. Are you sure you want to add customizer support for IE7?
#144
@
12 years ago
Nobody is saying anything about supporting the customizer in IE7. Let's stay on topic, please. This ticket is already quite long.
#146
follow-up:
↓ 147
@
12 years ago
Amazing progress on making Menus more usable. Have you considered something like this for the "manage screen"?
Allow people to drag their created menus into the theme's menu locations, instead of using dropdowns on its own panel. (We could also eventually include registered sidebars there, and automatically create a "custom menu widget" when you drag a menu to them.)
#147
in reply to:
↑ 146
@
12 years ago
Replying to matveb:
Amazing progress on making Menus more usable. Have you considered something like this for the "manage screen"?
Allow people to drag their created menus into the theme's menu locations, instead of using dropdowns on its own panel. (We could also eventually include registered sidebars there, and automatically create a "custom menu widget" when you drag a menu to them.)
A drag and drop interface was discussed on IRC, and dismissed as too clumsy and not terribly accessible, much like the current widgets interface.
#148
follow-up:
↓ 149
@
12 years ago
I apologize if the timing is off (too late?) but I believe there is another important UI shortcoming that I have not seen mentioned (in browsing this ticket).
The part of the screen that holds the UI containers from which menu content items can be selected is dysfunctional when there are many items (pages/posts/categories) to choose from ... especially when there are additional custom post-types and they too are populated with much content.
I wrote about this with what may be a direction to explore to make it better:
http://www.odharma.com/2010/07/review-of-wordpress-menus-user-interface/
#149
in reply to:
↑ 148
;
follow-up:
↓ 150
@
12 years ago
Replying to iamronen:
I wrote about this with what may be a direction to explore to make it better:
http://www.odharma.com/2010/07/review-of-wordpress-menus-user-interface/
Hey, thanks for chiming in. It's never too late (until we hit code freeze ;-)). I actually played around with a similar concept a while back: http://davemart.in/?p=74.
The big drawback to this approach that I saw (aside from it being a drastically different flow from what users are used to - and as a result something they would have to likely re-teach themselves), is that you lose the ability to add multiple pages, categories, or posts at once. In the current flow, you can select a bunch of pages, and add them all at once. With the flow you propose, you'd have to add each page one by one. I feel like that is a blocker for going this route.
#150
in reply to:
↑ 149
;
follow-up:
↓ 151
@
12 years ago
Replying to lessbloat:
The big drawback to this approach that I saw (aside from it being a drastically different flow from what users are used to - and as a result something they would have to likely re-teach themselves), is that you lose the ability to add multiple pages, categories, or posts at once. In the current flow, you can select a bunch of pages, and add them all at once. With the flow you propose, you'd have to add each page one by one. I feel like that is a blocker for going this route.
- My main point is that the second largest (if not largest) UI real-estate area should go to the "things I can add to the menu".
- The tabbed area makes it possible to allocate more real-estate to each of the "types of things" that can be added to the menu. I see no need to have on screen at the same multiple sources. If I am looking for posts - then let me see just posts. When I want to select from pages or categories or anything else I don't need to scroll down to look for them ... and to lose sight (literally) of the menu I am working on.
- Within the tabbed area there is then plenty of space that can be used for better search and filtering.
- Because the spaces are tabbed it may be possible to do on demand loading of data (instead of loading everything, everytime the screen loads). That may drastically improve page performance ...
- AND you can use that improved performance to do a smarter load ... like indicating somehow which items have already been added to the menu.
- You can still add checkboxes for selecting multiple items and an "add to menu" button. However what I tried to suggest was one click add instead of two click add. Instead of clicking to select each menu and then having to move the moust somewhere else AND clicking one more time "add to menu" you simply add directly. It feels to me like a faster, smoother and more efficient experience (unless I am missing something).
#151
in reply to:
↑ 150
;
follow-ups:
↓ 153
↓ 155
@
12 years ago
Replying to iamronen:
It feels to me like a faster, smoother and more efficient experience (unless I am missing something).
Let's try and keep this thread on task please. I do appreciate your input. But, the mockup you presented has a number of shortcomings, the largest of which I already mentioned (it being a drastically different flow from what users are used to). Which no amount of reasoning can remedy.
#152
follow-up:
↓ 154
@
12 years ago
I mocked up my interpretation of Nacins concept here. I'd love to hear your thoughts, but let's keep feedback for that design over on that thread for now.
#153
in reply to:
↑ 151
@
12 years ago
Replying to lessbloat:
I do appreciate your input. But, the mockup you presented has a number of shortcomings, the largest of which I already mentioned (it being a drastically different flow from what users are used to). Which no amount of reasoning can remedy.
@lessbloat, why not change the current flow? WordPress 3.5 changed drastically the media manager and the wordpress community embrace those changes. People love changes.
#154
in reply to:
↑ 152
@
12 years ago
Replying to lessbloat:
I mocked up my interpretation of Nacins concept here. I'd love to hear your thoughts, but let's keep feedback for that design over on that thread for now.
That mockup still does not address the most important area of the screen - the things I can pull into the menu. Try to populate the mockup with real content (put in a list of pages/posts/categories/custom post types ... and see that you simply can't.
#155
in reply to:
↑ 151
;
follow-up:
↓ 156
@
12 years ago
Replying to lessbloat:
Replying to iamronen:
It feels to me like a faster, smoother and more efficient experience (unless I am missing something).
Let's try and keep this thread on task please. I do appreciate your input. But, the mockup you presented has a number of shortcomings, the largest of which I already mentioned (it being a drastically different flow from what users are used to). Which no amount of reasoning can remedy.
This is off-topic (but I will not pursue it further then this short comment), but I think it is important. Is there a way/place/opportunity in the WordPress development process to debate your position? I'm OK with you asking me to back off ... really ... but is that the best thing for a design effort?
#156
in reply to:
↑ 155
;
follow-up:
↓ 157
@
12 years ago
Replying to iamronen:
This is off-topic (but I will not pursue it further then this short comment), but I think it is important. Is there a way/place/opportunity in the WordPress development process to debate your position? I'm OK with you asking me to back off ... really ... but is that the best thing for a design effort?
It's funny you should ask. Earlier today, @lessbloat proposed three approaches for this nav-menus action on the make.wordpress.org/core P2 site. Your two cents on that post are welcomed and encouraged.
Once the scope has been decided, we'll nail down the UI/workflow and work on revising our documentation. You can find and comment on @lessbloat's post here: http://make.wordpress.org/core/2013/01/23/for-tomorrows-menu-office-hours-i-only/#more-2813
#157
in reply to:
↑ 156
;
follow-up:
↓ 158
@
12 years ago
Replying to DrewAPicture:
Earlier today, @lessbloat proposed three approaches for this nav-menus action on the make.wordpress.org/core P2 site. Your two cents on that post are welcomed and encouraged.
Seems like the core P2 does not post my comments, so i will write my response here.
The current menu screen has too many functions (add new menus, edit existing menus, manage manus, assign menus to theme locations). Its all mixed up in one screen. Very confusing. This is why i prefer the two screens approach.
Not to mention that the "two screens approach" very similar to the post/page UI, which makes it much easier to use.
#158
in reply to:
↑ 157
@
12 years ago
Replying to ramiy:
Seems like the core P2 does not post my comments, so i will write my response here.
I don't see any comments from you awaiting moderation and other people are commenting just fine. Can you try to figure out what's going wrong there? This ticket is getting extremely long and difficult to follow, and it would be sad to lose your comment and its context.
#159
@
12 years ago
Replying to iamronen:
Is there a way/place/opportunity in the WordPress development process to debate your position?
Absolutely. For clarity's sake:
- I'm really not trying to be a jerk. (honest!) :-)
- This ticket is not the best place to post drastically new ideas.
- Even though I'm the lead for this feature, I don't have final say on any of it (that is left to Mark and Aaron). My role is to assist in the development, but also to keep the development of this feature on track, and on time.
We've got office hours for this feature each Mon & Thu at 21:00 UTC (4:00pm EST, 1:00pm PST) in #wordpress-dev. After we've covered any agenda items, you are free to bring up whatever ideas & concerns that you may have. There is also the make/core P2. You are welcome to leave whatever comments there that you'd like (as long as they are on topic). Those are your two best options, if you'd like to present a new idea.
#160
@
12 years ago
Some updated concepts posted here: http://make.wordpress.org/core/2013/01/23/for-tomorrows-menu-office-hours-i-only/#comment-7777 based on our discussion Thursday.
Please leave feedback on that thread. Thanks! :-)
#161
@
12 years ago
Here's my latest mockup proposal.
jkudish will gather some more data for tomorrows meeting (held this Thursday @ 21:00UTC in #wordpress-dev). We need finalize the direction we're taking with menus if this is going to make 3.6 - I hope to see you there.
#162
@
12 years ago
Still have js problems on P2 so here is what i think on the latest mockups:
The general direction is very good, selective menu screens depending on menu count is a very nice approach.
The new menu screen mockup is still overwhelming with management + editing + theme location assignment all in the same page. I personally prefer the two step workflow.
But if this is the preferred way, have you considered 3 columns menu screen?
We can add a screen option to hide the third column.
#163
@
12 years ago
23119.21.diff gets us back on track with:
Zero Menus State
When users have 1 theme location, and zero menus, and at least one page, we show them a simplified blank slate screen. We pre-populate it with the menu items that show up on their site by default (when no menus have been added).
Single Menu State
When users have one menu, we take them right to editing it (they don't see the menu management drop down):
Add Menu State
We reduced the amount of copy, and improved the visual hierarchy:
Multiple Menus State
When a user has more than one menu, we show them a drop down at the top where they can select another menu. This replaces the tabs approach which was confusing for users on multiple fronts:
Menu items
- We added keyboard accessibility for moving menu items
- We added "sub item" as helper text when menu items are dragged to a sub item position (multiple users had a positive reaction to this, as it improves clarity as to what just happened if they drag a menu item to a sub position by mistake).
Custom Links
Changed "Label" to "Link Text" (Before this change some users were not adding link text - since the change, everyone has).
Additional Improvements
- Improved & shortened copy in a bunch of places to provide better at-a-glance clarity
- Since the admin messages never fade away, some users were experiencing trouble knowing if their menus were saved (when they saved their menu multiple times in a row). We added in a slide down effect, which appears to have fixed the issue (we might consider doing this admin-wide).
#165
follow-ups:
↓ 166
↓ 167
@
12 years ago
In the Zero Menus State, I'm assuming from the presence of the "create menu" button that clicking that button (whether menu items are rearranged or not?) creates a new custom menu. Is that correct?
If so, I feel that may be a little confusing/non-obvious. Perhaps some copy explaining that this is the default menu (based on existing pages, right?), and that clicking the button saves it as a custom menu that can be edited, would be best?
#166
in reply to:
↑ 165
@
12 years ago
Replying to travisnorthcutt:
In the Zero Menus State, I'm assuming from the presence of the "create menu" button that clicking that button (whether menu items are rearranged or not?) creates a new custom menu. Is that correct?
Yes, that is correct.
If so, I feel that may be a little confusing/non-obvious. Perhaps some copy explaining that this is the default menu (based on existing pages, right?), and that clicking the button saves it as a custom menu that can be edited, would be best?
That's a great idea. Will do.
#167
in reply to:
↑ 165
@
12 years ago
Replying to travisnorthcutt:
If so, I feel that may be a little confusing/non-obvious. Perhaps some copy explaining that this is the default menu (based on existing pages, right?), and that clicking the button saves it as a custom menu that can be edited, would be best?
23119.22.diff changes it to: "Edit your default menu by adding or removing items. Drag each item into the order you prefer. Click Create Menu to save your changes." (suggestions welcome).
I'll start testing 23119.22.diff this afternoon, and will post the results to: http://make.wordpress.org/core/2013/02/01/we-had-a-great-discussion-yesterday-during-office/
p.s. Menus office hours will be today at 21:00 UTC (in #wordpress-dev). Swing past if you have questions/concerns.
#168
@
12 years ago
The idea behind the "zero menu" state is that it simplifies the flow for the majority of new users (based on the assumption that their theme has a single theme location).
Upon clicking "Create Menu" in the "zero menu" state, 23119.23.diff auto-saves the users new menu as their primary theme location. This saves users from having to manually select a menu for their single theme location (at this point we know they only have one of each), thus streamlining the flow, and consolidating two steps into one.
Users with themes that have zero theme locations, or more than one theme location will not see the zero menus state, and are taken right to the add new screen if they don't have any menus.
#169
@
12 years ago
I tested two more new users with 23119.23.diff applied. Overall, things went very smoothly, with only a couple of rough spots remaining.
Note: Here's our plan of attack for moving forward.
#170
follow-ups:
↓ 171
↓ 172
@
12 years ago
In 23119.24.diff I've added support for setting theme locations via checkboxes under 'Menu Settings'.
Note that this patch is sort of the minimal amount of work/code required to make this work, it can/needs to be cleaned up a bit and if we proceed with this approach we also need to remove the theme locations metabox code (which I've commented out the add_meta_box
line for now + added a todo for)
Here's how the checkbox looks:
Here's it looks if a menu is already assigned to a location (adds a note under the checkbox):
Let me know what you think and if you can test this, please do and let me know if there's any issues.
I also have 2 unrelated notes of something I noticed with this latest patch that we should probably fix:
- 'delete menu' link should be red
- there should be some explanatory text under 'Menu Structure' before you add items to it, otherwise it's a very confusing heading on it's own
#171
in reply to:
↑ 170
;
follow-up:
↓ 173
@
12 years ago
Replying to jkudish:
In 23119.24.diff I've added support for setting theme locations via checkboxes under 'Menu Settings'.
How will that work when there are many theme locations, a dozen or so? Yes, that’s rare, but then it will look rather confusing, I guess.
#172
in reply to:
↑ 170
@
12 years ago
Replying to jkudish:
In 23119.24.diff I've added support for setting theme locations via checkboxes under 'Menu Settings'.
Awesome. I'm excited to test this.
I also have 2 unrelated notes of something I noticed with this latest patch that we should probably fix:
- 'delete menu' link should be red
- there should be some explanatory text under 'Menu Structure' before you add items to it, otherwise it's a very confusing heading on it's own
Yep, I agree. both are valid, and should be addressed.
#173
in reply to:
↑ 171
;
follow-ups:
↓ 174
↓ 176
@
12 years ago
Replying to toscho:
How will that work when there are many theme locations, a dozen or so? Yes, that’s rare, but then it will look rather confusing, I guess.
Note: This is just a proposed concept (something we'll test with a couple users). If it flops in usability testing, we're back to square one.
The theme location checkboxes will just stack on top of each other. I would think a dozen would be extremely edge case. My guess is that the majority of themes have 1-3 theme locations. Thanks to Philip we have some data to back that up (my guess is that this same pattern is found in most wp.org themes as well).
I suppose we could add a hook in there, so theme devs could swap out the checkboxes for something else. But I suspect that if you're using a theme with 12 theme locations, you're a step above the average user, and you can likely figure out what's going on.
#174
in reply to:
↑ 173
@
12 years ago
Replying to lessbloat:
The theme location checkboxes will just stack on top of each other.
My mistake, jkudish coded them up horizontally:
Which I actually don't mind at all. I changed my mind after posting this. I feel like the checkboxes should stay stacked, as we're adding (currently uses 'Menu Name') in light gray next to one if it is used by another menu.
#175
@
12 years ago
The horizontal alignment of the checkboxes was an accidental ommision, I meant to have them stacked vertically. Will submit a revised patch in a bit.
#176
in reply to:
↑ 173
@
12 years ago
Replying to lessbloat:
The theme location checkboxes will just stack on top of each other. I would think a dozen would be extremely edge case.
For a regular blog, yes. When WP is used as a CMS, no. I had to do this multiple times: Custom menu locations for custom post types (sidebars in singular views) for custom roles and combinations of both.
I suppose we could add a hook in there, so theme devs could swap out the checkboxes for something else.
That would be very useful.
#177
@
12 years ago
demo_theme_locations.php is a micro-plugin to add 3 additional theme locations for testing.
#178
@
12 years ago
23119.24.1.diff just adds a few small tweaks:
- Enabled ability to add menu items in the "zero menu" state.
- Made delete link blue (it turns red on hover)
- Added explanatory text back under 'Menu Structure' before you add items
- If you create more than one menu, and Menu 1 is already assigned to a theme location, you'll see:
I'm open to copy/formatting suggestions for the "Set to" bit. We can also add a confirmation JS alert if they click the Primary box, asking if they're sure they want to switch the Primary theme location to this new menu (as it can only be set to one menu at a time).
#179
in reply to:
↑ 95
;
follow-up:
↓ 182
@
12 years ago
This is a monster trac at this point and I'm not entirely sure I'm following it all, but with regard to this:
Replying to lessbloat:
2) pressing the right, left, up or down key.
Supports RTL. Supports moving single item as well as items with children (sub items). Ideally, it should behave identical to drag/drop. With that said, there is a lot going on here, so please ping me if you notice any issues.
Can you please elaborate on this functionality? From the standpoint of a blind person, how would I be moving this item and know where I'm moving it? Moving something without vision, even while only using the keyboard, seems to me something that would inherently require vision.
Whereas, for instance, ordering items in the accessibility mode of the widget screen relies on numbers to delineate where the widgets would be.
#180
@
12 years ago
@lessbloat: Looks like 23119.24.1.diff needs to be regenerated. It didn't come through correctly.
#181
@
12 years ago
23119.25.diff:
- makes the delete link red (thanks Nacin)
- adds accordion styling to menu items (for testing)
- adds RTL CSS
- Removes headers in menu edit box (i.e. "Settings", and "Menu structure"), to reduce vertical space.
- Moves the menu name field back up to the top bar. This saves us vertical space, and allows us to put the settings down below the menu structure section.
Next, we'll run another round of usability testing, and then re-evaluate next steps based on the results.
#182
in reply to:
↑ 179
;
follow-up:
↓ 183
@
12 years ago
Replying to ceo:
This is a monster trac at this point
;-) Sorry, I take responsibility for that. I'm still learning how to use Trac effectively.
From the standpoint of a blind person, how would I be moving this item and know where I'm moving it? ... Whereas, for instance, ordering items in the accessibility mode of the widget screen relies on numbers to delineate where the widgets would be.
The only thing I've done is make it possible to rearrange the menu items via the keyboard (vs. requiring users to drag and drop menu items with a mouse). As you've pointed out, this improves accessibility for some users, but not all. To be honest, this is the first I've seen/heard of the accessibility mode in widgets.
For full disclosure, I'm really a novice when it comes to accessibility design. With that said, I'm eager to help where I can, so I appreciate your bringing this to my attention. A couple of questions:
- If I set you up with a sandbox for testing with the latest (23119.25.diff) patch applied, would you be wiling to make a list of all of the remaining in-accessible elements on the menu screen?
- In the widgets accessibility mode, is going to a new screen to re-assign order helpful in that it removes other distractions, or was it just coded up that way?
#183
in reply to:
↑ 182
@
12 years ago
Replying to lessbloat:
For full disclosure, I'm really a novice when it comes to accessibility design. With that said, I'm eager to help where I can, so I appreciate your bringing this to my attention. A couple of questions:
- If I set you up with a sandbox for testing with the latest (23119.25.diff) patch applied, would you be wiling to make a list of all of the remaining in-accessible elements on the menu screen?
Yes, provided you walk me through what I'd need to do on my end. The extent of my testing has been on nightly builds.
- In the widgets accessibility mode, is going to a new screen to re-assign order helpful in that it removes other distractions, or was it just coded up that way?
I don't know; I wasn't part of testing this when it was being introduced.
#184
follow-up:
↓ 185
@
12 years ago
In working on the 'Locations in the dropdown' task, I noticed a bug with 23119.25.diff where theme locations can only be assigned for one of your menus at a time.
I made a short screencast to illustrate the problem.
http://screencast.com/t/kySUEuVR6U
- First I assigned the 'Primary Menu' location to the 'Main Menu' menu.
- Next, I assigned the 'Location 1' location to the 'Something' menu. Notice that upon saving, there is no longer a notation that 'Primary Menu' location is assigned to 'Main Menu'.
- Revisited the editing screen for 'Main Menu' menu and note that 'Primary Menu' location is unchecked and 'Location 1' is assigned to 'Something' menu.
- Last, I overrode the setting for 'Location 1' location for the 'Main Menu' menu. This works as expected and overrides the setting for the current menu.
#185
in reply to:
↑ 184
@
12 years ago
Replying to DrewAPicture:
In working on the 'Locations in the dropdown' task, I noticed a bug with 23119.25.diff where theme locations can only be assigned for one of your menus at a time.
Oh that's a pretty bad bug. Thanks I'll work on a fix right now.
#186
@
12 years ago
Fixed the bug that DrewAPicture found and improved the menu saving code in 23119.26.diff
#187
@
12 years ago
Begininning to see the fruit of our labor in the latest round of usability tests. :-)
#188
@
12 years ago
23119.27.diff adds the locations in a comma-separated list to the 'Selected menu' dropdown. I'd love to iterate the foreaches around L#462.
Also note:
- I moved the
$locations
and$menu_locations
bits up to about L#395 just below where we set$_nav_menu->truncated_name
so I could reuse the arrays previously reserved for the locations checkboxes around L#555.
#190
@
12 years ago
- 23119.28.1 fixed some spacing issues around the locations dropdown foreach
- 23119.28.2 added more complete docs to new functions
- 23119.28.3 limits the number of locations in the dropdown to a filterable default of 3 (per yesterday's office hours) with ellipses. Looks like this:
Wasn't sure if I should do ", ...
" or just " ...
". English majors?
#191
follow-ups:
↓ 192
↓ 193
@
12 years ago
Some no-js observations:
- There's no way to select menus from the drop-down. Perhaps a no-js button?
- Header backgrounds in the accordion boxes are inconsistent due to
.closed
class being applied inconsistently. - Pointer needs to be switched to default in accordion box headers (fixed in 23119.28.4)
- Extra 31px padding-top from #menu-settings-column (fixed in 23119.28.4)
- There's a yellow "pending" menu-items message that shows reminding you to save. Might be worth looking at showing that regardless of js/no-js
Unrelated no-js nice-to-have:
- Kinda lame that there's currently no way to make menu items into sub-items. Can only move them up and down.
General:
- The 'Reset', formerly 'Cancel' link in menu-items has no effect in both js and no-js.
#192
in reply to:
↑ 191
;
follow-ups:
↓ 194
↓ 206
@
12 years ago
Replying to DrewAPicture:
Some no-js observations:
- There's no way to select menus from the drop-down. Perhaps a no-js button?
It's difficult for me to fully comment on functionality from just a screenshot, but for accessibility reasons it is always desirable to have a 'Go' button of some sort when a select or drop-down is being used as a navigational device. Where the page or context changes purely triggered by the onchange event, this means that keyboard only users (including screen readers) will be triggering such functionality as they roll through the options in the select.
#193
in reply to:
↑ 191
@
12 years ago
Thanks for reviewing those DrewAPicture.
Replying to DrewAPicture:
Unrelated no-js nice-to-have:
- Kinda lame that there's currently no way to make menu items into sub-items. Can only move them up and down.
The down/up arrows should handle this as well (though it's not super intuitive). Just continue clicking an arrow in the same direction. At least that's the way it used to work.
General:
- The 'Reset', formerly 'Cancel' link in menu-items has no effect in both js and no-js.
I'll look at this.
#194
in reply to:
↑ 192
@
12 years ago
Replying to grahamarmfield:
For accessibility reasons it is always desirable to have a 'Go' button of some sort when a select or drop-down is being used.
Good call. I'll get that added in place of the current auto-redirect code. Thanks grahamarmfield!
#195
@
12 years ago
Changes in 23119.29.diff include:
- Removed hacked in accordion code
- Re-hid the menu select drop down when users only have one menu. It should only show when users have more than one menu.
- Added a "Go" button for the menu selection drop down, and removed the auto-redirect code that was there.
- Changed "Reset" back to "Cancel", and fixed it so that it works.
- Hid "Cancel" link in no-js mode.
- Updated notification message to show in no-js mode.
- Reviewed CSS for this patch - moved all colors code to colors-fresh.css
- Cross browser testing (Tested in Mac FF, Chrome, Safari & Win IE8, FF, Chrome)
It's code review time. If you've got some time I'd appreciate the help.
#196
@
12 years ago
AaronCampbell did a quick at-a-glance code review, and found the following:
- Looks like an accidental replacement of @uses with wp_n in the doc block
- There's some commented out code in wp-admin/includes/nav-menu.php that should probably be removed instead of commented
- There's a small amount of trailing whitespace, which should be removed
- There are some strings that aren't translatable in wp-admin/nav-menus.php on line 451 (Menus, and Add New) and one line 483 (Go). There may be more, but those are the ones I saw
- In wp-admin/nav-menus.php on line 464 you're using esc_html in an attribute. Use esc_attr instead. This could happen other places, this is the one I noticed
Which I fixed in 23119.29.1.diff.
Additional code reviews would be much appreciated (from multiple people). Thanks for your help!
#197
follow-up:
↓ 199
@
12 years ago
It looks like there's still some trailing whitespace. For example, wp-admin/js/nav-menu.js line 399 has only changed because there's a space at the end now. Whoever commits can probably fix it, but it's likely that you can set your editor to trim trailing whitespace automatically.
#198
@
12 years ago
23119.29.2.diff fixes some coding standards on the new stuff, makes hex codes 6-digits on the CSS colors and some other small stuff.
Not sure I'm onboard with the 'Go' button on the dropdown select. I like that we have the button, but not 'Go'. I also think 'Selected menu' should change to maybe just 'Select menu', especially if we're adding a button. Thoughts?
#199
in reply to:
↑ 197
@
12 years ago
Replying to aaroncampbell:
It looks like there's still some trailing whitespace. For example, wp-admin/js/nav-menu.js line 399 has only changed because there's a space at the end now. Whoever commits can probably fix it, but it's likely that you can set your editor to trim trailing whitespace automatically.
Thanks for the tip. Found a plugin for Coda. As far as I can tell, 23119.29.3.diff should do the trick.
#201
in reply to:
↑ 200
@
12 years ago
Replying to wonderboymusic:
my favorite plugin for Coda is called "Netbeans"
Heh... Nice.
As the 200th comment on this ticket you win the grand prize!!!
Congratulations wonderboymusic!
You get the honorary privilege of code reviewing 23119.29.3.diff!!!
;-)
#202
@
12 years ago
__('Add ') . $tax->labels->name
is a no-go for translations. Needs sprintf() and _x().
#203
@
12 years ago
Code reviewed, and latest changes posted in 23119.30.diff. Here are my notes:
- added missing
absint()
when saving menu items - made some minor code standards/spacing adjustments (both in php and in js)
- adjusted the way we count the number of existing pages to use
wp_count_posts()
instead ofget_pages()
for performance reasons - using
absint()
when getting the recently edited menu instead of just(int)
, just to be safe - using
empty()
instead of just!
when checking if there is a recently edited menu, just to be safe - using
empty()
instead of comparing against0
when checking if there is a currently selected menu - making use of
selected()
in the new menu selection dropdown instead of doing anif
dance and manually echoingselected=selected
- replaced a URL that we were manually building with
?
to useadd_query_arg()
instead - addressed @ocean90's comment about the tax label name
- removed some unneeded
remove_query_arg()
- fixed incorrect usage of
esc_attr_e()
- added missing escaping on one of the submit buttons
- js: cached
$(this)
in a few spots that we missed it - js: yoda conditions
- Note: I did not review the css
- Note: Between
wp-admin/nav-menus.php
andwp-admin/includes/nav-menu.php
we have a lot of code. We should probably take a closer look to see if there's any repetition, unused code and such to see if we can optimize a bit. This seemed like beyond the scope of this initial code review, so I didn't dig too deep for now.
#204
follow-up:
↓ 205
@
12 years ago
Bug: Click the Add New button or go directly to wp-admin/nav-menus.php?action=edit&menu=0. Now you have changed your mind and want to change the selected menu. Do it via the dropdown menu.
=> Only the Menu Name will change, but the items are still missing.
Not sure, but is wp_nav_menu_disabled_check()
still needed? If so, the disabled() function should be called with the third argument, defined as false. Otherwise it will echoing the disabled attribute.
#205
in reply to:
↑ 204
@
12 years ago
Replying to ocean90:
Not sure, but is
wp_nav_menu_disabled_check()
still needed? If so, the disabled() function should be called with the third argument, defined as false. Otherwise it will echoing the disabled attribute.
I wondered about that when I was writing docblocks for it, especially since we removed the meta box. But it was either A) new or B) moved so I added the docs.
#206
in reply to:
↑ 192
;
follow-up:
↓ 207
@
12 years ago
Replying to grahamarmfield:
Replying to DrewAPicture:
Some no-js observations:
- There's no way to select menus from the drop-down. Perhaps a no-js button?
It's difficult for me to fully comment on functionality from just a screenshot, but for accessibility reasons it is always desirable to have a 'Go' button of some sort when a select or drop-down is being used as a navigational device. Where the page or context changes purely triggered by the onchange event, this means that keyboard only users (including screen readers) will be triggering such functionality as they roll through the options in the select.
grahamarmfield, it seems like the redirect on change would only take place upon selection of a select option, not just in browsing the select options. Can you please clarify which is the case for screen readers?
#207
in reply to:
↑ 206
;
follow-up:
↓ 208
@
12 years ago
Replying to lessbloat:
grahamarmfield, it seems like the redirect on change would only take place upon selection of a select option, not just in browsing the select options. Can you please clarify which is the case for screen readers?
For most keyboard only users, when encountering a dropdown they would use the up and down arrow keys to review the options. This (I believe) fires the onchange event whenever a new option is moved to. So hence my comment re context change triggered by onchange.
There is a keystroke combination in Windows that allows the dropdown to be opened and the options to be reviewed without firing the onchange event until the enter key is pressed. But this keystroke combination is not widely known and is therefore likely to be hardly used.
So if the change of context is not triggered by the onchange event, but instead by tabbing away from the dropdown (onblur?) then I guess we're into a bit of a grey area.
The issue with not providing a 'Go' or 'Select' (or whatever) button is not just a problem for keyboard and screen reader users. Those using speech recognition software also need to be catered for.
#208
in reply to:
↑ 207
@
12 years ago
Replying to grahamarmfield:
For most keyboard only users, when encountering a dropdown they would use the up and down arrow keys to review the options. This (I believe) fires the onchange event whenever a new option is moved to. So hence my comment re context change triggered by onchange.
In most browsers it is Shift+Arrow
. But you have to know in advance when you need that.
So if the change of context is not triggered by the onchange event, but instead by tabbing away from the dropdown (onblur?) then I guess we're into a bit of a grey area.
Would still not be a predictable behavior.
The issue with not providing a 'Go' or 'Select' (or whatever) button is not just a problem for keyboard and screen reader users. Those using speech recognition software also need to be catered for.
A button is also easier to use on a touch screen, because there is enough space around.
#209
@
12 years ago
23119.31.diff changes include:
- Changes copy around menu selection drop down
- Changed "Select menu to edit" container from span to label
- Changes "Go" button to "Select"
- Fixed "add new screen" bug mentioned in comment:204
#211
@
12 years ago
So I guess lessbloat has been manually incrementing his patch numbers. Just so you know, if you upload 23119.diff over and over, Trac will change it to 23119.2.diff, 23119.3.diff, etc.
Anyway, 23119.diff is based off 23119.31.refresh.diff. The only thing I did is take out the "Add %s" strings where %s was a post type name or taxonomy name. Strings like that aren't translatable. If it turns out that we need a string like that, it would need to be added to the labels array in get_taxonomy_labels()
and get_post_type_labels()
. Since it doesn't currently exist though, many plugins and themes won't be using it and a default might be worse than not having the word Add prepended.
#213
follow-up:
↓ 214
@
12 years ago
Notice: Undefined offset: 0 in wp-admin/nav-menus.php on line 475
#214
in reply to:
↑ 213
@
12 years ago
Replying to scribu:
Notice: Undefined offset: 0 in wp-admin/nav-menus.php on line 475
Mind expanding on what steps produced the notice?
#215
@
12 years ago
Can't reproduce it anymore. Must have been some weird edge case when transitioning from the old screen.
#216
follow-up:
↓ 217
@
12 years ago
- Keywords needs-patch added; has-patch commit removed
What is the benefit of changing the metabox title Custom Link to Add Links? These are all links which can be added to a menu.
The prefix Add should be removed to be consistent with the other titles. IMO Custom should come back too, since there is still a way to get the old Link Manager back, which isn't meant here.
#217
in reply to:
↑ 216
;
follow-ups:
↓ 218
↓ 219
@
12 years ago
Replying to ocean90:
What is the benefit of changing the metabox title Custom Link to Add Links? These are all links which can be added to a menu.
The prefix Add should be removed to be consistent with the other titles. IMO Custom should come back too, since there is still a way to get the old Link Manager back, which isn't meant here.
One of the things we're still implementing is making the menu items into an accordion. This improves the experience by decreasing the chance that the user will have to scroll to access all of the menu item options.
One downside of having most of the menu items closed on the left side is that it becomes harder for a first time user to know what is going on there. "Custom link" doesn't really convey any action. By switching it to "Add links" I felt like the underlying functionality became more discoverable at a glance, and the concept became much more clear (that each of the accordion options on the left allow me to add menu items).
An alternative could be to add a header above the accordion, something like: "Add menu items". This would be less redundant in the use of the word "Add", but it would push the accordion options down (slightly), and make for much less of a balanced UI (right now it's squared off nicely with a straight line across the top of the menu items accordion box, all the way across to the menu editing box). Happy to take a stab at it, if we think that approach would be better.
#218
in reply to:
↑ 217
@
12 years ago
Replying to lessbloat:
"Custom link" doesn't really convey any action.
How about "Add a Custom Link"?
#219
in reply to:
↑ 217
;
follow-up:
↓ 235
@
12 years ago
Replying to lessbloat:
By switching it to "Add links" I felt like the underlying functionality became more discoverable at a glance, and the concept became much more clear
But then the question remains, why just for Links?
#220
follow-up:
↓ 221
@
12 years ago
I'm seeing the notice Scribu had too. It's in the selectbox. Most likely because of the existing menu.
There is still going on something weird with that. I had to create a new menu first before I could look at the old one. It was just showing the page to create a new menu.
#221
in reply to:
↑ 220
@
12 years ago
Replying to markoheijnen:
I'm seeing the notice Scribu had too. It's in the selectbox. Most likely because of the existing menu.
There is still going on something weird with that. I had to create a new menu first before I could look at the old one. It was just showing the page to create a new menu.
Was the existing menu assigned to a theme location?
#222
@
12 years ago
Good question. I don't know. The only thing I know is that it isn't anymore. Setting or unsetting the theme location doesn't make a difference
#223
@
12 years ago
RE: the notice from comment:213 / comment:220:
Notice: Undefined offset: 0 in wp-admin/nav-menus.php on line 475
After a conversation in IRC, @markoheijnen pasted a var_dump of $menu_locations
:
<DrewAPicture> markoheijnen: Can you share a var_dump on $menu_locations from before the foreach? <markoheijnen> array(2) { <markoheijnen> [0]=> <markoheijnen> int(2) <markoheijnen> ["primary"]=> <markoheijnen> int(2) <markoheijnen> }
My best guess is there's a problem with set_theme_mod()
setting an invalid, absint-ed menu location with a value of 0, which therefore translates into an undefined offset into the second foreach for the select box.
#224
follow-up:
↓ 232
@
12 years ago
I was getting a different error:
Warning: array_merge(): Argument #1 is not an array in /wp-admin/nav-menus.php on line 277
Which was caused by $menu_locations
being set to false when it was empty. 23119.3.diff fixes both the notices that @markoheijnen and I were seeing
#225
follow-up:
↓ 226
@
12 years ago
23119.3.diff does the trick for the undefined offset and array_merge errors.
Also to note, there is a case of an "invisible" menu on a vanilla install. When you first visit Menus and create a new menu, it's saved but you're sent back to the Create Menu screen as though you don't have any menus.
As @jkudish notes, it's likely caused by $nav_menu_selected_id
being empty without a menu_id in the URL.
#226
in reply to:
↑ 225
@
12 years ago
Replying to DrewAPicture:
23119.3.diff does the trick for the undefined offset and array_merge errors.
Also to note, there is a case of an "invisible" menu on a vanilla install. When you first visit Menus and create a new menu, it's saved but you're sent back to the Create Menu screen as though you don't have any menus.
As @jkudish notes, it's likely caused by
$nav_menu_selected_id
being empty without a menu_id in the URL.
23119.4.diff (which includes 23119.3.diff) should fix the "invisible" menu issue and make sure that we always display a menu if one exists.
#227
follow-up:
↓ 236
@
12 years ago
23119.4.diff does indeed fix the "invisible" menu problem.
Also two things:
- The UX is inconsistent for creating your first menu and creating a new menu. I feel like they should be the same or nearly so.
- Creating your first menu looks like this: http://cl.ly/image/2m0P3X0X0l2i
- Creating a new menu looks like this: http://cl.ly/image/0w20023K320F
- Perhaps we should look at opening a new ticket and/or individual tickets for new stuff cropping up such as refactoring and/or docs, etc. When we hit what would be 23119.7, the incrementer will restart at 23119.32
#228
follow-up:
↓ 229
@
12 years ago
I totally misunderstood the UI here and couldn't figure out how to create a new menu. However, it was obvious once someone pointed it out to me. I'm unsure if this was just me being an idiot, or if there is a genuine UX problem. Currently I'm erring on the side of "Ryan being an idiot", but thought I'd point out my difficulties in case it has any impact on decisions here.
#229
in reply to:
↑ 228
;
follow-up:
↓ 237
@
12 years ago
Replying to ryanhellyer:
I totally misunderstood the UI here and couldn't figure out how to create a new menu. However, it was obvious once someone pointed it out to me. I'm unsure if this was just me being an idiot, or if there is a genuine UX problem. Currently I'm erring on the side of "Ryan being an idiot", but thought I'd point out my difficulties in case it has any impact on decisions here.
I agree with you. Even though having the 'Add Menu' button up in the header is consistent with the other UI screens, I feel like it's really not very discoverable up there. @markoheijnen didn't see it right away the other day either.
I'd be curious to see if user tests will confirm there's a discoverability problem.
#230
@
12 years ago
The most funny part with me was that I did add some menu's before but after removing all menu's I somehow couldn't see it anymore.
I disagree that is would be consistent with other UI screens. Those screens are just overviews so the call to action is completely different.
#234
@
12 years ago
- Keywords needs-patch removed
- Resolution set to fixed
- Status changed from new to closed
Please open new tickets for any new issues.
#235
in reply to:
↑ 219
@
12 years ago
Replying to ocean90:
Replying to lessbloat:
By switching it to "Add links" I felt like the underlying functionality became more discoverable at a glance, and the concept became much more clear
But then the question remains, why just for Links?
I had "Add" in there for all of them. Looks like they were ripped out for translation purposes.
#236
in reply to:
↑ 227
@
12 years ago
Replying to DrewAPicture:
23119.4.diff does indeed fix the "invisible" menu problem.
Also two things:
- The UX is inconsistent for creating your first menu and creating a new menu. I feel like they should be the same or nearly so.
- Creating your first menu looks like this: http://cl.ly/image/2m0P3X0X0l2i
- Creating a new menu looks like this: http://cl.ly/image/0w20023K320F
Open to additional thoughts on this, but this was actually intentional. For your first menu we preload all of the menu items that show in your theme by default (before you add our first menu). We do this out of convenience. When you click the "add new" link, I don't think we should preload any menu items. If you'd like to add a menu for a menu widget, you should be able to do so without first having to remove a bunch of menu items.
- Perhaps we should look at opening a new ticket and/or individual tickets for new stuff cropping up such as refactoring and/or docs, etc. When we hit what would be 23119.7, the incrementer will restart at 23119.32
Agreed.
#237
in reply to:
↑ 229
@
12 years ago
Open to suggestions for the "add new" link. My feeling was that we should keep it consistent with the rest of the add new links in the admin.
23119-A.diff is in the same vein as #22991, but it:
It looks like this:
Note: The blank gap to the left of the new drop down will be filled by the intro sentence in D.