-
Notifications
You must be signed in to change notification settings - Fork 384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add sanitizer to fixup issues with core themes #1074
Changes from 1 commit
8643b8b
4692577
51eb633
fc0bea1
7f738c1
cbacd9d
f186cd9
31cb154
a36a281
f5ca657
601c005
80e2355
dc9e628
54e025a
65aec1e
a91f0e2
e47275c
d41e498
d3b8eae
d2247ff
3bb3dd8
ac98dfc
93c87eb
7b9acc6
df2e561
bca1ead
e1fc251
d55cf6a
425c80a
70d1440
2cdb1c8
1fdb325
2fc015d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
<?php | ||
/** | ||
* Class AMP_Core_Theme_Sanitizer. | ||
* | ||
* @package AMP | ||
* @since 1.0 | ||
*/ | ||
|
||
/** | ||
* Class AMP_Core_Theme_Sanitizer | ||
* | ||
* Fixes up common issues in core themes and others. | ||
* | ||
* @since 1.0 | ||
*/ | ||
class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { | ||
|
||
/** | ||
* Config for features needed by themes. | ||
* | ||
* @var array | ||
*/ | ||
public $theme_features = array( | ||
'twentyseventeen' => array( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a great idea to pass an array of arguments to each of the methods, depending on the theme. |
||
'force_svg_support' => array(), | ||
'force_fixed_background_support' => array(), | ||
// @todo Header video probably needs special support in the plugin generally. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #1078 |
||
// @todo Header image is not styled properly. Needs layout=responsive and other things? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #1094 (comment) |
||
// @todo Dequeue scripts and replace with AMP functionality where possible. | ||
), | ||
); | ||
|
||
/** | ||
* Fix up core themes to do things in the AMP way. | ||
* | ||
* @since 1.0 | ||
*/ | ||
public function sanitize() { | ||
$theme_features = array(); | ||
|
||
// Find theme features for core theme. | ||
$theme_candidates = wp_array_slice_assoc( $this->args, array( 'stylesheet', 'template' ) ); | ||
foreach ( $theme_candidates as $theme_candidate ) { | ||
if ( isset( $this->theme_features[ $theme_candidate ] ) ) { | ||
$theme_features = $this->theme_features[ $theme_candidate ]; | ||
break; | ||
} | ||
} | ||
|
||
// Allow specific theme features to be requested even if the theme is not in core. | ||
if ( isset( $this->args['theme_features'] ) ) { | ||
$theme_features = array_merge( $this->args['theme_features'], $theme_features ); | ||
} | ||
|
||
foreach ( $theme_features as $theme_feature => $feature_args ) { | ||
if ( method_exists( $this, $theme_feature ) ) { | ||
call_user_func( array( $this, $theme_feature ), $feature_args ); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Force SVG support, replacing no-svg class name with svg class name. | ||
* | ||
* @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/assets/js/global.js#L211-L213 | ||
* @link https://caniuse.com/#feat=svg | ||
*/ | ||
public function force_svg_support() { | ||
$this->dom->documentElement->setAttribute( | ||
'class', | ||
preg_replace( | ||
'/(^|\s)no-svg(\s|$)/', | ||
' svg ', | ||
$this->dom->documentElement->getAttribute( 'class' ) | ||
) | ||
); | ||
} | ||
|
||
/** | ||
* Force support for fixed background-attachment. | ||
* | ||
* @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/assets/js/global.js#L215-L217 | ||
* @link https://caniuse.com/#feat=background-attachment | ||
*/ | ||
public function force_fixed_background_support() { | ||
$this->dom->documentElement->setAttribute( | ||
'class', | ||
$this->dom->documentElement->getAttribute( 'class' ) . ' background-fixed' | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be great to eventually go back and add tests for this class, and other new methods.