WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#30553 closed defect (bug) (fixed)

Use Customizer deep-links inside the theme modal

Reported by: ocean90 Owned by: ocean90
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.1
Component: Customize Keywords: has-patch
Focuses: Cc:

Description

Follow-up to #25569 and #25571

Attachments (3)

30553.png (16.2 KB) - added by ocean90 6 years ago.
30553.diff (2.0 KB) - added by DrewAPicture 6 years ago.
30553.2.diff (3.2 KB) - added by ocean90 6 years ago.

Download all attachments as: .zip

Change History (11)

@ocean90
6 years ago

@DrewAPicture
6 years ago

#1 @DrewAPicture
6 years ago

  • Keywords has-patch added; needs-patch removed

Added 30553.diff for deep-linking the Header and Background buttons. It ain't pretty but it should maintain the status quo as far as that science experiment of a code block goes :-)

#2 follow-up: @celloexpressions
6 years ago

This'll also need to be updated in the Underscore JS template further down in that file. I really don't think we need to deep-link these here, as they extra links add a lot of clutter, and Customize is the primary action. Can we add .hide-if-customize to the existing links, but not change the URLs? That way they're only there if we have to link to the old pages and there is no Customize link, and we can reduce the clutter if they can Customize.

#3 in reply to: ↑ 2 @DrewAPicture
6 years ago

Replying to celloexpressions:

This'll also need to be updated in the Underscore JS template further down in that file. I really don't think we need to deep-link these here, as they extra links add a lot of clutter, and Customize is the primary action. Can we add .hide-if-customize to the existing links, but not change the URLs? That way they're only there if we have to link to the old pages and there is no Customize link, and we can reduce the clutter if they can Customize.

Two issues here:

  1. Not deep-linking creates two clicks instead of one. If we really are "replacing" those header and background screens with controls in the Customizer then you need to account for that from all angles
  2. We need to avoid making an assumption that people will just realize that those options are now available in the Customizer. They won't.

#4 @celloexpressions
6 years ago

In this context, though, I think we can make those assumptions (personal opinion). But if others disagree we would just do the extra links with the alternating hide if/no customize classes. It's important that the old links remain for users that can't access the Customizer.

@ocean90
6 years ago

#5 @ocean90
6 years ago

30553.2.diff needs some testing. Not sure if the $class stuff is even needed.

#6 @DrewAPicture
6 years ago

30553.2.diff seems to work as expected. Not even sure what the significance of the $class stuff was prior to this change, for what that's worth.

#7 @ocean90
6 years ago

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

In 30793:

Themes: Use Customizer deep-links inside the theme modal.

props DrewAPicture, ocean90.
see #25569, #25571.
fixes #30553.

#8 @ocean90
6 years ago

In 30794:

Remove a stray $ from [30793].

props ipm-frommen.
see #30553.

Note: See TracTickets for help on using tickets.