WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#38399 closed defect (bug) (fixed)

Twenty Seventeen: Improve front page "panels" to follow core best practices

Reported by: celloexpressions Owned by:
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: dev-feedback ux-feedback
Focuses: Cc:
PR Number:

Description

Here's the start of what we'd need to do to make the current approach appropriate for a bundled theme in terms of the official customize API guidelines and ideal UX:

  • Remove the panel. Themes are strongly discouraged from using panels, especially when they're generic "theme options", and sections should always have multiple controls when possible. All of the controls in the panel belong in a single section.
  • Consider moving the layout control to a separate section, or if it stays in theme options, consider adding at least one other related option so we have multiple controls in this section. Or, remove this control entirely. It seems like the two-column layout is signature of this theme, so it seems a bit odd to have an option to turn it off.
  • If the layout control stays, it needs to update with postMessage, by toggling a body class.
  • Consider moving the front page content controls to the static front page section.
  • Replace the arbitrary number of panels with a dynamic approach. In the customizer, this could be done with a single dropdown-pages control (which will soon be able to create pages), and a minimal custom control in the theme that shows the selected items and allows them to be removed. This would be stored as an array of featured page ids, similar to the proposed core approach. For simplicity in the theme, reordering wouldn't be supported, but we'd get a lot of the UX ideas developed for core carried over.
  • Remove the borders around sections - they prevent users from seeing the design as it will be published.
  • Implement selective refresh for the front page content, which will bring visual edit icons as well.

Once we get into it we'll likely have more issues come up, and some of those things can potentially be skipped, but this is a rough list of what we need to work through if it stays in the theme. We should not merge the theme into core with the current solution as-is because it violates the best practices with panels that we're trying really hard to encourage themes to follow.

Change History (7)

#1 @karmatosed
3 years ago

Noting there is a lot of discussion and back story on this here: https://github.com/WordPress/twentyseventeen/issues/348.

Potentially we should split this into separate tickets - that could help a lot as there's a lot in one. I would leave that up to you @davidakennedy though how you want it split. I'm just porting tickets over at this point after merge.

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

#2 @davidakennedy
3 years ago

I'm cool with leaving this as one ticket and working through it.

I'd also add this to the list: Making the panels dry, reported by @davidmosterd

See: https://github.com/WordPress/twentyseventeen/pull/308

I originally closed it, but both @davidmosterd and @celloexpressions had some good points on a rework. So let's have a patch for that, but with a comment or two clarifying what's happening for new themers.

#3 @celloexpressions
3 years ago

  • Milestone 4.7 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Closing in favor of #38426, which addresses this holistically alongside other related aspects.

#4 @karmatosed
3 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

I'm not sure that a totally different way covers this. Seems parts in here we want to also explore - we could of course open new tickets for those.

I would be keen to keep this open so we can discuss the other elements and not see one way as the only solution yet.

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

#5 @celloexpressions
3 years ago

  • Milestone set to 4.7

Let's revisit this ticket to continue iterating after the initial adjustments in #38426 are resolved, then. The primary necessary changes in that patch (for code and UX standards and due to integration with other changes) are:

  • Making the code DRY
  • Adding selective refresh support (+ visual edit icons, with #27403)
  • Restructuring the controls into a single section, as sections are intended to group multiple related controls. That also takes care of the layout option that's completely unrelated to this.

It also contains adjustments to the communications between the controls and the preview that are necessary as a part of the above points; this is the area with the most potential for finding a middle ground and includes things like the border highlights.

This ticket was mentioned in Slack in #core-themes by celloexpressions. View the logs.


3 years ago

#7 @celloexpressions
3 years ago

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

Fixed in [38386]. See #38408 for the colors.

Note: See TracTickets for help on using tickets.