WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 17 months ago

#27406 closed task (blessed) (fixed)

Widget Customizer: Organize all widget area sections into a meta customizer section

Reported by: westonruter Owned by:
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.9
Component: Customize Keywords: has-patch
Focuses: ui, javascript Cc:

Description

Customizer sections for widget areas only appear on previewed URLs that contain that widget area (sidebar) used in the template (i.e. dynamic_sidebar() is used). By hiding the sections for widget areas not shown in the preview, we can conserve a lot of space in the customizer. If all widget area sections are always shown, the sidebar would be very cluttered.

A solution to this is that all the widget area sections should be grouped into one meta section, as shaunandrews has proposed:

https://github-camo.global.ssl.fastly.net/0228089e384833d5df07f0f7dd9540555af6778d/68747470733a2f2f692e636c6f756475702e636f6d2f76514b53415666586c4c2e676966

With this in place, more widget area sections could be shown at a time, and potentially make it easier to reorder widgets between sidebars as well, as right now it is a bit hacky how drag-and-drop support is implemented to move widgets between customizer sections, as normally only one section is open at a time in the accordion.

Some messaging could be shown to direct users that they should navigate to the page that has the sidebar in use and then enter the customizer from there. They can either navigate to this page from within the customizer or from the frontend and click the link in the admin bar. There could also be a checkbox/button to toggle between showing only the widget areas currently used in the previewed URL, or to show all of the widget areas regardless but with some additional indicator (such as being faded out, see also #27405).

Originally discussed at https://github.com/x-team/wp-widget-customizer/issues/77

Attachments (15)

27406.diff (10.8 KB) - added by celloexpressions 3 years ago.
Proof-of-concept for customizer "Pages"
27406.1.diff (11.2 KB) - added by celloexpressions 3 years ago.
Proof-of-concept for customizer "Pages", puts widgets into a page.
27406.2.diff (13.1 KB) - added by celloexpressions 3 years ago.
Pages are sub-accordions, UI improvements, code cleanup from @westonruter.
27406.2.parents.diff (11.7 KB) - added by celloexpressions 3 years ago.
Alternate API approach, using sections with children instead of a separate WP_Customize_Page object.
27406.3.pages.diff (13.6 KB) - added by celloexpressions 3 years ago.
Minor CSS fix, remove debug, for "Pages" version.
27406.4.diff (15.0 KB) - added by celloexpressions 3 years ago.
"Panel," improved animations, hide close button, details
27406.alwaysclose.diff (959 bytes) - added by celloexpressions 3 years ago.
Always close all accordion sections when moving in and out of panels.
27406.accordion.js-docs.diff (3.5 KB) - added by celloexpressions 3 years ago.
Cleanup old/unneeded functions, add some inline documentation and an example to accordion.js.
27406.keyboard-accessibility.diff (2.3 KB) - added by celloexpressions 3 years ago.
Fix keyboard accessibility in panels by removing hidden elements from focus and moving the focus as needed.
27406.scroll-position-fixes.diff (1.4 KB) - added by celloexpressions 3 years ago.
Fix panel positioning when not scrolled to top.
27406.accordion.js-docs.2.diff (3.7 KB) - added by celloexpressions 3 years ago.
2nd pass at accordion.js docs and cleanup.
27406.3.diff (3.0 KB) - added by helen 3 years ago.
27406v4.diff (496 bytes) - added by MattyRob 3 years ago.
27406.5.diff (1.5 KB) - added by michalzuber 3 years ago.
Missing text for translation
27406.disallow-single-section-panels.diff (1011 bytes) - added by celloexpressions 3 years ago.
If a panel has only one section, display the section in the top level with the panel context in its title.

Download all attachments as: .zip

Change History (85)

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


3 years ago

#2 @Kniebremser
3 years ago

+ 1

I use a theme that offers me 9 different places for widgets.
This makes it really packed full and cluttered.
I find the solution best.

#3 @nacin
3 years ago

FWIW, I do still love this. Was any code ever done?

#4 @ocean90
3 years ago

  • Keywords needs-patch added
  • Type changed from feature request to enhancement

Talked a bit with nacin.

There is a POC from shaunandrews: http://www.shaunandrews.com/themes/new-customizer/

We want to give this a chance and need a patch which does the following:

  • It should be only available for widgets.
  • Only option should be to manage widgets or return (not like this mockup, where you see the save/cancel button)
  • It should show *all* sidebars.
  • Sidebars, which aren't used on the current preview page should be semi-transparent, this will make #27405 invalid, which is okay.

Sure it's a bit late in cycle, but it's worth at least seeing if it's small and also a clear/obvious improvement. Since we want this anyway, at worst the patch will find it's way into 4.0 if we agree it's an improvement.

This ticket was mentioned in IRC in #wordpress-dev by ocean90. View the logs.


3 years ago

#6 @westonruter
3 years ago

In terms of how to go about implementing this from a backend perspective, as noted in that GitHub issue above:

Currently there is a separate customizer control for each sidebar and for each widget in each sidebar. So there is a mapping of one setting per control. However, if we were to merge the two controls into one widget area control, and associated multiple settings with that one control, then we'd be able to just have one single "Widgets" customizer section, and this section would be populated with these Widget Area controls. Each widget area control then could have each individual widget control appear inside of it: so we'd have our 3 levels. And when expanding the "Widgets" section, we could make it fly out as shown in @shaunandrews's prototype, instead of expand down as normal.

As for how to implement the UI for this, I suppose it would require overriding the default behaviors attached to expanding the accordion customizer section for widget area, to cause the widget section content to take over the entire panel instead of expanding the accordion section content below.

#7 @adamsilverstein
3 years ago

We discussed this a bit in IRC (ref) and I started working on an implementation that would add a new 'page' feature to the customizer, essentially allowing you to add sections to a specific 'page' (or pane) of the customizer.

Adding a page would create the top level page link (and sliding functionality) and sections placed on a page would be added to a div that could then slide into and out of place.

This should yield a clean implementation and leave much of the widget customizer as is and could get some other use in the future. Its a bit to build! I have a good start and will report back here with progress.

Last edited 3 years ago by adamsilverstein (previous) (diff)

#8 follow-up: @nacin
3 years ago

I think it's great to have a plan (and I like the plan). I will say one thing: I don't really care how "clean" the implementation is for 3.9. If anything, it'd probably be *better* if it was completely private/internal and specific to widgets, that way we can take the time to get an API right (not to mention the UX). Otherwise everyone will suddenly start converting to these before we're even ready. "pane" probably makes more sense, terminology-wise. But even then I don't want to think about it so much; I want 3.9 to be shippable. :-) RC by end of week is the goal.

Separately, I'd like to emphasize something that ocean90 touched on. If this does seem to be too much (UX-wise, code-wise, whatever), too much change late in the game, or it's not a clear and obvious improvement, then it's going to have to wait for 4.0. That's OK, as it means we're still going to try to use it. That would give us extra time for user testing, which is good.

One thing to try, given the short timeline, is to upload work-in-progress patches often, even if broken, barely working, or otherwise incomplete, so we can get an idea as early as possible how things are going and how we feel about them.

Thanks for picking this up, Adam!

#9 in reply to: ↑ 8 @adamsilverstein
3 years ago

I agree this is coming too late for 3.9 - 3.9.1 or 4.0 is fine. I think its a clear improvement, but it will take some effort to get it right, get it tested, etc. and its not something we need for 3.9 - the widget customizer works well without this addition.

As soon as I get a little bit further I will upload a work in progress patch as you suggested so others can weigh in on my proposed approach.

Replying to nacin:

I think it's great to have a plan (and I like the plan). I will say one thing: I don't really care how "clean" the implementation is for 3.9. If anything, it'd probably be *better* if it was completely private/internal and specific to widgets, that way we can take the time to get an API right (not to mention the UX). Otherwise everyone will suddenly start converting to these before we're even ready. "pane" probably makes more sense, terminology-wise. But even then I don't want to think about it so much; I want 3.9 to be shippable. :-) RC by end of week is the goal.

Separately, I'd like to emphasize something that ocean90 touched on. If this does seem to be too much (UX-wise, code-wise, whatever), too much change late in the game, or it's not a clear and obvious improvement, then it's going to have to wait for 4.0. That's OK, as it means we're still going to try to use it. That would give us extra time for user testing, which is good.

One thing to try, given the short timeline, is to upload work-in-progress patches often, even if broken, barely working, or otherwise incomplete, so we can get an idea as early as possible how things are going and how we feel about them.

Thanks for picking this up, Adam!

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


3 years ago

#11 follow-up: @celloexpressions
3 years ago

I really like this. I'm going to need to do something along these lines for my Menu Customizer GSoC project, as users will have the ability to create new menus (creating new customizer sections). Generally, having a core API for this should make the customizer easier to streamline while new features are added by both core and plugins and themes.

@adamsilverstein - what's the current status here, do you have a work-in-progress patch? I'm happy to help with this however I can, although I'm probably most suited to testing and implementing it for menus. We should get going on this soon if we want to try to get it into 4.0.

#12 in reply to: ↑ 11 @westonruter
3 years ago

Replying to celloexpressions:

I really like this. I'm going to need to do something along these lines for my Menu Customizer GSoC project, as users will have the ability to create new menus (creating new customizer sections). Generally, having a core API for this should make the customizer easier to streamline while new features are added by both core and plugins and themes.

What do you think about my suggestion above to refactor the Widget Customizer to instead have a single customizer section instead of one customizer section for each widget area? In this scheme, each widget area would be a customizer control and then the widget controls would be pseudo controls inside of the widget area.

But I suppose there is also value in also making available a concept of paging for customize sections. But it seems that what we're really talking about here is the introduction of Customizer Subsections. There would still need to be a top-level section (e.g. “Widgets”) that would be clicked to enter into the page of customzier subsections.

In terms of the API, the args passed to $wp_customize->add_section() could then include a parent arg which would refer to another customizer section. When selecting the parent section, then the top-level sections could slide off of the customizer panel to then list the customizer subsections, as depicted in the mock above.

The introduction of subsections could provide a path forward to logically group top-level sections into logical categories (e.g. Colors) to reduce the increasing overload in the number of customizer sections registered.

#13 follow-up: @westonruter
3 years ago

If Customizer Subsections were implemented, what would happen to controls that are added to the parent Customizer Section? Would that be disallowed or would such controls appear after (or before) the subsections are listed?

#14 in reply to: ↑ 13 ; follow-up: @celloexpressions
3 years ago

Replying to westonruter:

If Customizer Subsections were implemented, what would happen to controls that are added to the parent Customizer Section? Would that be disallowed or would such controls appear after (or before) the subsections are listed?

I've been thinking of it as Customizer Supersections, I suppose for this reason. Those could be special sections that, rather than containing controls, contained sections. In the UI they would slide over and only show the sections in that super-section, as in the mockup.

For the API, it would probably be best to have something like $wp_customize->add_supersection() or $wp_customize->add_page(), which behaved similarly to $wp_customize->add_section(). And then we would have something like a parent arg, as you said, for add_section that assigned that section as a sub-section of the super-section. I would probably look for a better name than parent, as that potentially implies the ability to nest further levels of depth, which I think would probably get out of control fairly easily.

So basically, sections and controls remain how they are now, but a new level of hierarchy would be added above some sections. Sections that don't have parents would be displayed alongside supersections in the top-level view.

#15 in reply to: ↑ 14 ; follow-up: @westonruter
3 years ago

Replying to celloexpressions:

I've been thinking of it as Customizer Supersections, I suppose for this reason. Those could be special sections that, rather than containing controls, contained sections. In the UI they would slide over and only show the sections in that super-section, as in the mockup.

It seems to me that subsections are more in line with other constructs in WP core. For example, posts can have other posts as parents, terms have term parents, and admin bar nodes have node parents. However, a "customizer supersection" would be a completely different thing than a "customizer section".

Last edited 3 years ago by westonruter (previous) (diff)

#16 in reply to: ↑ 15 @celloexpressions
3 years ago

Replying to westonruter:

It seems to me that subsections are more in line with other constructs in WP core. For example, posts can have other posts as parents, terms have term parents, and admin bar nodes have node parents. However, a "customizer supersection" would be a completely different thing than a "customizer section".

I think there are two potential implementation approaches:

  • Parent and children sections, both are WP_Customize_Section, but with special handling for items that have children. Matches several existing constructs in core. Need to figure out whether parent sections can have both sections and controls as direct "children". Children defined with a parent arg.
  • Something like WP_Customize_Page which would extend WP_Customize_Section but be a collection of sections instead of controls. Fits with the relationship of controls being children of sections, but not with other core constructs. Children defined with a page-like arg. Need to determine the best name for this.

Basically, there's going to be a layer of parent/children-type items sharing a hierarchical level either way. Either sections and pages are displayed together or sections and controls are.

@celloexpressions
3 years ago

Proof-of-concept for customizer "Pages"

@celloexpressions
3 years ago

Proof-of-concept for customizer "Pages", puts widgets into a page.

#17 follow-ups: @celloexpressions
3 years ago

  • Component changed from Widgets to Appearance
  • Keywords has-patch dev-feedback needs-testing added; needs-patch removed

27406.1.diff is a proof-of-concept patch for introducing customizer "Pages" and putting all sidebars into a page. This implements the second approach I outlined above. Themes and plugins can add their own pages via $wp_customize->add_page() and $wp_customize->add_section( $id, array( 'title' => '...', 'page' => $page ) );.

While pages and top-level sections could be combined in sorting with this approach, I kept them separate, meaning all pages would be displayed either above or below all top-level sections. I don't think we should enforce anything like requiring all sections to be in pages.

I'd be interested in seeing other possible approaches and/or improvements on this approach. And also thoughts on the UX. The patch is designed to be user-testable and the UI could be considered complete other than some sort of a sliding animation. My patch could use work on the JS side especially. It looks like I broke widget-adding too, but don't have time to dig into that at the moment (looks like a minor issue, though, I didn't spend much time trying to disable the conditional sidebar display part).

#18 @westonruter
3 years ago

I applied the patches to a clone of git.develop.wordpress.org and opened a PR so that the work can be more easily code reviewed, and so that additional commits can be pushed or so that the repo can be forked: https://github.com/x-team/wordpress-develop/pull/13

#19 in reply to: ↑ 17 @westonruter
3 years ago

Replying to celloexpressions:

While pages and top-level sections could be combined in sorting with this approach, I kept them separate, meaning all pages would be displayed either above or below all top-level sections. I don't think we should enforce anything like requiring all sections to be in pages.

I do think that the arrays for pages and sections should be merged together and sorted by their respective priorities.

#20 in reply to: ↑ 17 @westonruter
3 years ago

Replying to celloexpressions:

My patch could use work on the JS side especially. It looks like I broke widget-adding too, but don't have time to dig into that at the moment (looks like a minor issue, though, I didn't spend much time trying to disable the conditional sidebar display part).

It seems to work just fine for me. I was able to add new widgets to the sidebars.

I added a couple commits to strip off the "Widgets:" prefix from the widget area sections, and to make the "Widgets" page title translatable, among some other tweaks.

#21 @westonruter
3 years ago

I think the customizer accordion should be cleaned up to support nested sections, so that the sections within a page can be output as child elements of the accordion-section-content, i.e.:

/**
 * Render the page, and the sections that have been added to it.
 *
 * @since 4.0.0
 */
protected function render() {
	?>
	<li id="accordion-section-<?php echo esc_attr( $this->id ); ?>" class="control-section control-page accordion-section">
		<span class="preview-notice"><?php _e( 'You are editing your site&#8217;s' ); ?></span>
		<h3 class="accordion-section-title" tabindex="0"><?php echo esc_html( $this->title ); ?></h3>
		<ul class="accordion-section-content">
			<?php if ( ! empty( $this->description ) ) : ?>
				<li><p class="description"><?php echo $this->description; ?></p></li>
			<?php endif; ?>
			<?php
			foreach ( $this->sections as $section ) {
				$section->maybe_render();
			}
			?>
		</ul>
	</li>
	<?php
}

This would make it much easier to show/hide the pages since they'd all be logically contained within a common parent ul element, following the page's description if supplied.

@celloexpressions
3 years ago

Pages are sub-accordions, UI improvements, code cleanup from @westonruter.

#22 @celloexpressions
3 years ago

  • Keywords ux-feedback added

We've been iterating on this on github and 27406.2.diff contains the current progress. The UI/UX is now fairly polished, with support for page descriptions and an arrow (modeled after the theme browser arrows) to go back to the main Customizer page. Now would be a good time for UI/UX people to weigh in; I put up a screencast demoing it here: https://cloudup.com/cpFF42uiW2l.

This still needs some decisions, and feedback from lead devs on the API side. My earlier comment still applies, with the current patch taking the second approach. I'll look at an implementation with the first approach as well for easier comparison. Once that's sorted out, depending on which way we go, we can make final decisions about how to handle sorting, etc.

@celloexpressions
3 years ago

Alternate API approach, using sections with children instead of a separate WP_Customize_Page object.

#23 @celloexpressions
3 years ago

27406.2.parents.diff implements the alternate approach, using parent sections instead of a separate page object. This gives us support for sections and parent sections being sorted in the same group of priorities and support for controls as direct children of parent sections/pages without additional work.

However, after doing both implementations I prefer the version with WP_Customize_Page (27406.2.diff) for the following reasons:

  • Once you play with it, it's pretty clear that supporting controls that are direct children results in a really uncomfortable UX, because it feels like the controls aren't in a section (and they aren't really, once you enter the sub-section view).
  • It will be necessary to add the parent section separately anyway, so using add_page() versus add_section() gives the advantage of clearer code without extra steps (see the changes to widgets between the two patches for comparison). You don't have to look to see if other sections are referencing a given section as a parent to determine if it's a super-section or a regular one.
  • We can (and should consider) group top-level-sections and pages together for sorting by priority in the page approach; and could look into a WP_Customize_Group class that WP_Customize_Section and WP_Customize_Page both extend (@westonruter's idea).

I should note that the UI is the same in both patches, with the exception of the sorting of pages/parent-sections.

@celloexpressions
3 years ago

Minor CSS fix, remove debug, for "Pages" version.

#24 @westonruter
3 years ago

  • Milestone changed from Future Release to 4.0

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.


3 years ago

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


3 years ago

#27 @adamsilverstein
3 years ago

Very nice! This worsk just like I imagined it should and is a big improvement. Thanks!

Tiny detail: I see a little visual artifact as I switch to the widget page, the arrow in the upper right next to the theme slides to the left (screenshot)

Version 1, edited 3 years ago by adamsilverstein (previous) (next) (diff)

#28 @celloexpressions
3 years ago

Working on a new patch that uses the term "panel" instead of "page", per IRC, and makes several other tweaks ranging from improved keyboard accessibility to keying colors to color schemes and hiding the close button when inside a sub-panel. I'll post it once I figure out a better way to fix the imperfections with the sliding animation.

@celloexpressions
3 years ago

"Panel," improved animations, hide close button, details

#29 follow-ups: @celloexpressions
3 years ago

In 27406.4.diff:

  • Use the term "Panel" instead of page, for better clarity with the API, per IRC.
  • Hide close button when in panels, per IRC. Currently this allocates enough space for several translations that I checked, but it may need a bit more for others.
  • The phrase "You are editing your site's ..." has been replaced with "You are customizing ...", which should work for most use-cases for panels (this is in the panel title area).
  • Improve the animations sliding into and out of panels. It should now be a simple, smooth slide left/right, with the arrow and close button doing similar.
  • Add a brief description to the "Widgets" panel, based on the help text in the Widgets admin page.
  • Key the colors to color schemes where needed (just the back arrow for now).

Other notes:

  • #28504 would introduce icons to customizer sections. I'd propose only including icons on panels and top-level sections, but leaving them off of sub-sections within panels (maybe including the panel header one for the whole panel), as all sections within a panel should theoretically have a similar purpose.
  • Should all panels be listed before top-level sections, or should they have mixed sorting? I think it's better to keep them separate, as the small arrow indicators are more effective when grouped, and users are probably more likely to need to use a panel then a top-level section when entering the Customizer at any given time (if Widgets and Menus are panels, for example).
  • The keyboard accessibility needs some work, I'm not sure what the best behavior would be but it technically works in.4.diff (have to tab back up to the top arrow to exit a sub-panel).
  • It would be cool if we automatically built in a deep-linking mechanism to access specific panels immediately by their id when entering the customizer (future enhancement).

Overall, this is getting pretty polished and may be ready for a first pass to go in soon.

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


3 years ago

#31 in reply to: ↑ 29 @ethitter
3 years ago

I'm glad the Panel approach was chosen over the parent approach. Each top-level Customizer item is already a series of interconnected classes with an implied hierarchy. Implementing a Panel as yet another class in this chain reinforces the existing developer experience.

Replying to celloexpressions:

  • #28504 would introduce icons to customizer sections. I'd propose only including icons on panels and top-level sections, but leaving them off of sub-sections within panels (maybe including the panel header one for the whole panel), as all sections within a panel should theoretically have a similar purpose.

I agree, there's no need to repeat the icons within panels.

  • Should all panels be listed before top-level sections, or should they have mixed sorting? I think it's better to keep them separate, as the small arrow indicators are more effective when grouped, and users are probably more likely to need to use a panel then a top-level section when entering the Customizer at any given time (if Widgets and Menus are panels, for example).

Sections already support priority. Since a Panel is an extension of the WP_Customize_Section class, shouldn't it respect priority?

#32 follow-up: @helen
3 years ago

Really, really loving this. The "panel" concept makes a lot of sense to me. Agree that it's very close to first run - everything else I'm going to list can be done post-first-pass, just would like confirmation of review from another committer who's deeper in this (cough, ocean90).

Some thoughts on testing the patch:

  • Keyboard accessibility - I think escape should take you back until you can't go back anymore, and then close the customizer (I don't believe it does this now). I believe the back button achieves the same effect - we may want to do this with history, perhaps (see: deep-linking).
  • Also on accessibility - should have somebody with a screen reader (or various screen readers) evaluate how various elements announce themselves, such as back/close. This will almost certainly have to happen after first run, as it's easier to get somebody testing trunk as opposed to a patch.
  • Not sure about grouping panels vs. sections - I can see how it would make sense to group them together, but the ordering feels wrong with Widgets at the top.
  • The arrow, while large at first, quickly felt right, and made me wonder if the "close" button should then become an X instead, with the arrow sliding over it. You could quickly get out of the customizer that way - click a couple times in the same spot.
  • Deep-linking would be super awesome. :)
  • How does the "You are customizing X" string work with translations, since the design assumes the X will be the last word? I guess we do the same with "You are previewing X", mostly just curious/cautious.

#33 in reply to: ↑ 29 @westonruter
3 years ago

Replying to celloexpressions:

It would be cool if we automatically built in a deep-linking mechanism to access specific panels immediately by their id when entering the customizer (future enhancement).

Filed ticket for this in #28650: Allow Customizer elements (controls, sections, and panels) to be deep-linked

#34 @ocean90
3 years ago

In 28861:

Customizer: Introduce a "panel" API to organize multiple sections into a one section.

Create a panel via $GLOBALS['wp_customize']->add_panel( $panel_id, $args ) and use $panel_id for the panel argument in $GLOBALS['wp_customize']->add_section( $section_id, $args ). That's it.
As an example all widget area sections are now summarized into one panel. Feedback appreciated.

props celloexpressions.
see #27406.

#35 in reply to: ↑ 32 @ocean90
3 years ago

  • Keywords dev-feedback removed

Replying to helen:

How does the "You are customizing X" string work with translations, since the design assumes the X will be the last word? I guess we do the same with "You are previewing X", mostly just curious/cautious.

Changed this a bit in [28861] to match "You are previewing X".

#36 @DrewAPicture
3 years ago

  • Keywords needs-docs added

The inline PHP docs in [28861] look like a pretty good start, and I'd like to see more inline JS docs here too. This especially as (correct me if I'm wrong) some of the accordion code is shared with menus. Any takers?

#37 @celloexpressions
3 years ago

  • Close button -> X: created #28655 for discussion so that this ticket doesn't become too overwhelming.
  • accordion.js inline docs: I can do those, I've gotten pretty acquainted with that file. I believe that the idea is for this file to be used for any accordions needed throughout the admin (currently only used for Menus), either where the basic accordion classes are used or where do_accordion_sections is used, per #23449. Unfortunately I think this is the best place to put the panels JS, but everything still functions fine for normal (and nested) accordions. There is also some stuff in there that was needed for pre-mp6 styling, I can take that out at the same time.
  • Accessibility: it looks like esc and backspace don't do anything currently; I think backspace should always control the previewed url, via #28536, and then we could use esc for the controls. Tabbing through, the biggest keyboard issues are that the back buttons are always focus-able, resulting in an extra tab between panel titles in the top level, and when reaching the end of a panel, the focus continues to tab through hidden controls. Additionally, the focus styling is horribly inadequate throughout the customizer accordion. This may be a good time to do a more comprehensive customizer accessibility review and iterations, should we move that to a new ticket?
  • Sorting/priorities: in the current implementation, all panels are displayed by their priorities and then all sections are displayed by their priorities. Until there are more panels in core, that does mean that widgets will often be alone at the top (pending theme/plugin panels). We certainly could do mixed priorities, although that would be a bit less elegant, as we may need two copies of everything in WP_Customize_Manager (panels, sections, and the combined array, perhaps groups). I think we should think about this from a UI/UX perspective and get some feedback. For me, having the panels at the top makes sense because the types of things that'll go in panels will generally be more-commonly-used (ie, users change their widgets more frequently than their site title/tagline, header/background, colors, static front page, etc.). Could probably get some stats from .com to back that up (even though their customizer UI is fairly different).
  • Mentioned this in IRC, but we should try to get both #27981 and #28477 into 4.0 also so that all of these API improvements can happen together to make it easier for devs to track.

I believe that covers all of the currently outstanding issues that haven't been extracted to other tickets.

#38 follow-up: @ocean90
3 years ago

  • Keywords needs-patch added; has-patch removed

Minor issue:

  • Open Customizer
  • Go to the Widgets panel
  • Open first section
  • Click back arrow
  • Go to the Widgets panel again
  • You will see that the first section is still open, try to close it

Expected: Section closes
Is: Section will be still open

#39 @ocean90
3 years ago

In 28931:

Customizer: Reverse arrows in RTL. See #27406.

props yoavf.
fixes #28669.

@celloexpressions
3 years ago

Always close all accordion sections when moving in and out of panels.

@celloexpressions
3 years ago

Cleanup old/unneeded functions, add some inline documentation and an example to accordion.js.

#40 in reply to: ↑ 38 @celloexpressions
3 years ago

  • Keywords has-patch added; needs-patch removed

Replying to ocean90:

Minor issue:

  • Open Customizer
  • Go to the Widgets panel
  • Open first section
  • Click back arrow
  • Go to the Widgets panel again
  • You will see that the first section is still open, try to close it

Expected: Section closes
Is: Section will be still open

That was actually intentional - each accordion panel acted independently. However, there was some residual code that looked like it was supposed to prevent that, it was buggy, and I can see how it would be better to always close all sections when going in/out of panels, so 27406.alwaysclose.diff should do the trick.

I also did some improved inline docs for accordion.js including a file header with an example. I'm not sure exactly what we should do here, as the JS isn't as structured as the parts of core that have JS docs already, but this should be sufficient for anyone looking into what the different parts of this file do. I also removed some unneeded pre-mp6-specific code. That's all in 27406.accordion.js-docs.diff.

@celloexpressions
3 years ago

Fix keyboard accessibility in panels by removing hidden elements from focus and moving the focus as needed.

#41 @celloexpressions
3 years ago

27406.keyboard-accessibility.diff fixes keyboard accessibility in and around panels. Users can tab through all elements that are visible in the currently visible context without hitting any hidden elements, and focus is automatically moved to visible elements when moving in and out of panels.

This should cover everything needed for keyboard accessibility for now; additional improvements, while needed, should be habdled in other tickets as this patch brings the accessibility up to the rest of the Customizer's current state.

#42 @helen
3 years ago

  • Type changed from enhancement to task (blessed)

@celloexpressions
3 years ago

Fix panel positioning when not scrolled to top.

#43 @celloexpressions
3 years ago

27406.scroll-position-fixes.diff fixes vertical positioning issues when entering a panel from a scroll position other than the top in the top-level Customizer controls.

These accordion.js patches may or may not conflict with each other all over the place, I haven't checked. They're probably all equal priority, and are all necessary, though they each address different issues.

#44 @DrewAPicture
3 years ago

The docs in 27406.accordion.js-docs.diff look pretty good. Looks like you missed the @param tags for accordionSwitch() and panelSwitch(). Also need @since tags for both.

@celloexpressions
3 years ago

2nd pass at accordion.js docs and cleanup.

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.


3 years ago

@helen
3 years ago

#46 @helen
3 years ago

In 29035:

Customizer panels:

  • Close all accordion sections when going in and out of panels.
  • Fix panel positioning when top-level Customizer controls are scrolled down.
  • Keyboard accessibility: only focus on visible elements and transfer between them as needed when navigating around panels.

props celloexpressions. see #27406.

#47 @helen
3 years ago

In 29036:

accordion.js:

  • Remove remnants from when some corners were rounded.
  • Docs.

props celloexpressions. see #27406.

#48 @celloexpressions
3 years ago

  • Keywords needs-testing needs-docs has-patch removed

Only remaining item here, barring any bugs, is whether panels and top-level sections should have independent or mixed sortings. The current, separate approach is a cleaner implementation and asserts than panels contain more-commonly-used controls. It also makes it easier, as a user, to distinguish between panels that will completely change the Customizer's context and sections, which simply open inline. I imagine that would create a really strange user experience, unless there is a more obvious visual distinction between panels and top-level sections. Eventually, I could see a time when we may force all sections to be contained in a panel (placing unspecified ones into a generic panel), so we may want to consider that as well.

For now we're going to keep the separate sorting and we'll see what feedback we get during beta. Merging sorting wouldn't change the API or the UI implementation, other than what values one would use for the priority arguments, but would require a significant amount of work.

Last edited 3 years ago by celloexpressions (previous) (diff)

#49 @helen
3 years ago

In 29042:

Remove an unused JS variable after [29036]. see #27406.

#50 follow-up: @MattyRob
3 years ago

The most recent commit caused the wp-admin/js/accordian.js file to throw jshint errors. The attached patch fixes those be removing a now unused var.

@MattyRob
3 years ago

#51 @DrewAPicture
3 years ago

  • Keywords commit added

#52 in reply to: ↑ 50 @celloexpressions
3 years ago

  • Keywords commit removed

Replying to MattyRob:

The most recent commit caused the wp-admin/js/accordian.js file to throw jshint errors. The attached patch fixes those be removing a now unused var.

Looks like helen beat you to it by a couple minutes [29042] :)

@michalzuber
3 years ago

Missing text for translation

#53 follow-up: @ocean90
3 years ago

As mentioned by michalzuber "Back to Customize" needs to be translated. Not sure about the wording here, could just "Back" be enough?

#54 in reply to: ↑ 53 @celloexpressions
3 years ago

Replying to ocean90:

As mentioned by michalzuber "Back to Customize" needs to be translated. Not sure about the wording here, could just "Back" be enough?

"Back" would be better - we're now using the term "customize" in all panels, so "Back to Customize" doesn't make sense. Unless anyone has a better idea? I'm thinking this should probably be more specific, as it's only there for screen readers.

#55 @DrewAPicture
3 years ago

  • Keywords needs-patch added

"Back" should be fine.

Last edited 3 years ago by DrewAPicture (previous) (diff)

#56 @ocean90
3 years ago

In 29132:

Customizer: Make screen reader text translatable.

see #27406.

#57 @michalzuber
3 years ago

'You are customizing %s' should not be translatable?
Second change in https://core.trac.wordpress.org/attachment/ticket/27406/27406.5.diff

#58 @ocean90
3 years ago

In 29135:

Customizer: Make panel title translatable too.

props michalzuber.
see #27406.

#59 @DrewAPicture
3 years ago

In 29157:

Inline documentation cleanup for 4.0 audit.

phpDoc tweaks for methods added in [28861]:

  • WP_Customize_Manager::panels() method
  • WP_Customize_Manager::add_panel() method
  • WP_Customize_Manager::get_panel() method
  • WP_Customize_Manager::remove_panel() method

Added in [28970]:

  • WP_Customize_Manager::customize_preview_override_404_status() method

See #27406 and #28885.

#60 @DrewAPicture
3 years ago

In 29158:

Inline documentation cleanup for 4.0 audit.

phpDoc tweaks for methods added in [28861]:

  • WP_Customize_Panel constructor method
  • WP_Customize_Panel::render() method

See #27406 and #28885.

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


3 years ago

@celloexpressions
3 years ago

If a panel has only one section, display the section in the top level with the panel context in its title.

#62 @celloexpressions
3 years ago

  • Keywords has-patch added; needs-patch removed

I'm becoming more and more worried about people abusing panels with loads of unnecessary options. While we can't do much (and at least this is better than too many options without being organized into panels), we can educate developers on the primary UX purpose of panels as distinct contexts within the customizer (ex. widgets or menus).

We can also take some hints from other parts of the Customizer, where sections (and now panels) are not displayed if they don't contain any controls (/sections). When a panel contains only one section, that section might as well be displayed in the top-level, and the panel ignored. In most cases, I would say that panels should either contain a minimum of 3-4 different sections or have a very specific feature-related context (like widgets or menus), where controls and sections are dynamically created. However, the best approach that we could take, while maximizing user experience, is to reduce single-section panels to top-level sections.

27406.disallow-single-section-panels.diff automatically reduces panels that only contain one section to a top-level section. The panel title is prepended to the the section title to provide some context for generically-named sub-section titles. This is extremely useful for dynamically-generated panels such as widgets; note that single-widget-area themes will automatically skip the widgets panel with the patch applied, and that the patch doesn't touch widgets at all.

#63 @celloexpressions
3 years ago

#28979 was marked as a duplicate.

#64 @celloexpressions
3 years ago

  • Keywords ux-feedback removed

Moved sorting/priorities discussion to #28979 per @nacin, so only remaining thing here is 27406.disallow-single-section-panels.diff. After that, let's leave this open for any other issues/feedback until RC then can close in favor of new tickets.

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


3 years ago

#66 follow-up: @ocean90
3 years ago

To be honest I don't want to have a restriction as in 27406.disallow-single-section-panels.diff in core. It just feels wrong to educate people in this way.

#67 in reply to: ↑ 66 @celloexpressions
3 years ago

Replying to ocean90:

To be honest I don't want to have a restriction as in 27406.disallow-single-section-panels.diff in core. It just feels wrong to educate people in this way.

I should have clarified - while this would help reduce unnecessary panels/developer education, it's primarily for better user experience. And the customizer already imposes similar restrictions for developers, such as not rendering empty sections. It's easy enough to see that the panel part "worked" since the panel title is prepended to the section title.

Having a panel with only one section is never a good experience for users; that section might as well be displayed in the top level. It basically just introduces an extra click. In dynamic situations like widgets, only showing the panel if there ends up being only one section in it is not trivial. The idea here is to make things easier on developers by automatically adapting in situations where panels aren't actually needed.

Alternatively, we could implement something like 27406.disallow-single-section-panels.diff for widgets only, but that's not as straightforward to do and wouldn't pass the benefits on to other panels.

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


3 years ago

#69 @celloexpressions
3 years ago

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

No one else is excited about 27406.disallow-single-section-panels.diff, so there are no outstanding issues on this ticket.

I'm going to close this out - work continues on #28979; please open new tickets with any additional issues.

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.