WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20896 closed enhancement (fixed)

Theme Customizer: Add "no-customize-support" class to the body tag by default

Reported by: kobenland Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: has-patch
Focuses: Cc:

Description

I just stumbled across this comment for wp_customize_support_script():

It is also recommended that you add the "no-customize-support" class to the body tag by default.

And I thought: Let's!

Related: #20751, [20918]

Attachments (4)

20896.diff (601 bytes) - added by kobenland 3 years ago.
Add 'no-customize-support' class to the body tag by default
20896.2.diff (1.0 KB) - added by kobenland 3 years ago.
In JS: just replace the class don't add it, since it's already there.
20896.3.diff (506 bytes) - added by SergeyBiryukov 3 years ago.
20896.4.diff (535 bytes) - added by DrewAPicture 3 years ago.
body class

Download all attachments as: .zip

Change History (19)

@kobenland3 years ago

Add 'no-customize-support' class to the body tag by default

comment:1 @kobenland3 years ago

  • Component changed from General to Appearance

comment:2 follow-up: @scribu3 years ago

First of all, why? What does this patch fix?

@kobenland3 years ago

In JS: just replace the class don't add it, since it's already there.

comment:3 in reply to: ↑ 2 @kobenland3 years ago

Replying to scribu:

First of all, why? What does this patch fix?

In [20918] functionality was introduced to hide the 'Customize' link, when the customizer is not supported.

By adding 'no-customize-support' to the default body classes (when the admin bar is showing), we can be sure it is there and don't have to rely on Themes' best practice, we'd streamline behavior with the admin, where the class is also added by default and can shave off a bit from the JS.

comment:4 @nacin3 years ago

The reasoning behind removing the class was to specifically separate the loader from the admin. koopersmith probably remembers more.

comment:5 @obenland3 years ago

So this ticket can be closed as wontfix then, right?

comment:6 @obenland3 years ago

#22052 was marked as a duplicate.

comment:7 @SergeyBiryukov3 years ago

From #22052:

In order for hide-if-no-customize in the Toolbar to work, no-customize-support has to be added by default to the body classes. If there is Customize support, wp_customize_support_script() rewrites it to customize-support.

comment:8 @DrewAPicture3 years ago

  • Cc xoodrew@… added

The loader was split from wp_customize_support_script() in r20918, which allows us to add the body class now. Not sure about the changes proposed in theme.php but the body class alone sufficed in my testing. See #20751.

@SergeyBiryukov3 years ago

comment:9 @SergeyBiryukov3 years ago

In order for hide-if-no-customize in the Toolbar to work, no-customize-support has to be added by default to the body classes.

From my testing, this is exactly what wp_customize_support_script() does, on the front end as well. In IE 7, which doesn't support the customizer, the link in the Toolbar is not visible.

I guess the comment about adding no-customize-support class to the body tag by default is no longer relevant. 20896.3.diff removes it.

comment:10 follow-up: @nacin3 years ago

  • Keywords close added

Adding no-customize-support by default is to handle flashes of should-be-hidden content before the JS in wp_customize_support_script() fires. Otherwise, content that is both .hide-if-customize and .hide-if-no-customize might appear by default, among other side effects.

But, since "Customize" appears in a dropdown in the toolbar, and therefore hidden on load, we're not worried about a flicker, and can let the JS handle it.

@DrewAPicture3 years ago

body class

comment:11 in reply to: ↑ 10 ; follow-up: @DrewAPicture3 years ago

  • Keywords close removed

Replying to nacin:

Adding no-customize-support by default is to handle flashes of should-be-hidden content before the JS in wp_customize_support_script() fires. Otherwise, content that is both .hide-if-customize and .hide-if-no-customize might appear by default, among other side effects.

But, since "Customize" appears in a dropdown in the toolbar, and therefore hidden on load, we're not worried about a flicker, and can let the JS handle it.

My intent is to allow for a graceful no-js fallback. no-js means no Customizer and it also means we can't rely on the JS to add the correct class for us. This is accounted for (unintentionally?) in the two other instances of Customizer links in the Dashboard because no-customize-support is added in admin-header.php. 20896.4.diff adds the body class for the front-end.

comment:12 in reply to: ↑ 11 @koopersmith3 years ago

  • Keywords dev-feedback removed

Conceptually, this makes sense. The original patch (20896.diff) is solid.

Replying to DrewAPicture:

This is accounted for (unintentionally?) in the two other instances of Customizer links in the Dashboard...

It was intentional. Your reasoning is correct.

comment:13 @nacin3 years ago

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

In [22107]:

Add no-customize-support to the body classes when the toolbar is showing. Allows for 'Customize' to be hidden when JS is disabled. props obenland, DrewAPicture. fixes #20896.

comment:14 @DrewAPicture3 years ago

Related: #22103 - wp_customize_support_script() removes spaces separating classes

comment:15 @SergeyBiryukov3 years ago

  • Milestone changed from Awaiting Review to 3.5
Note: See TracTickets for help on using tickets.