Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add AMP theme support #1286

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add AMP theme support #1286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1791783
Add AMP theme support
westonruter May 1, 2018
b4d24c2
Fix phpcs errors unrelated to AMP changes
westonruter May 1, 2018
5f31822
Restore use of button for menu-toggle in AMP
westonruter May 3, 2018
0ec1648
Simplify state for siteNavigationMenu
westonruter Jun 11, 2018
ebed1b3
Remove amp theme support in favor of letting user opt-in via admin
westonruter Jun 11, 2018
0c8d535
Remove implementation of comments_live_list for now
westonruter Jun 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 53 additions & 2 deletions functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,23 @@ function _s_setup() {
'flex-width' => true,
'flex-height' => true,
) );

/*
* If you intend your theme to be used with the AMP plugin and make use of AMP components in your templates,
* then you should make sure your site is served in native/canonical AMP via:
*
* add_theme_support( 'amp' );
*
* If you implement <amp-live-list> in your comments.php then you should do:
*
* add_theme_support( 'amp', array(
* 'comments_live_list' => true,
* );
*
* Otherwise, a user of the AMP plugin can decide via an admin screen for whether or not they want to serve
* your theme's templates in AMP responses, either in native AMP (canonical URLs) or paired AMP modes
* (separate AMP-specific URLs).
*/
}
endif;
add_action( 'after_setup_theme', '_s_setup' );
Expand Down Expand Up @@ -117,10 +134,44 @@ function _s_widgets_init() {
add_action( 'widgets_init', '_s_widgets_init' );

/**
* Enqueue scripts and styles.
* Determine whether this is an AMP response.
*
* Note that this must only be called after the parse_query action.
*
* @link https://github.com/Automattic/amp-wp
* @return bool Is AMP endpoint (and AMP plugin is active).
*/
function _s_scripts() {
function _s_is_amp() {
return function_exists( 'is_amp_endpoint' ) && is_amp_endpoint();
}

/**
* Enqueue styles.
*/
function _s_styles() {
wp_enqueue_style( '_s-style', get_stylesheet_uri() );
}
add_action( 'wp_enqueue_scripts', '_s_styles' );

/**
* Enqueue scripts.
*
* This short-circuits in AMP because custom scripts are not allowed. There are AMP equivalents provided elsewhere.
*
* navigation:
* In AMP the :focus-within selector is used to keep submenus displayed while tabbing,
* and amp-bind is used to managed the toggled state of the nav menu on small screens.
*
* skip-link-focus-fix:
* This is not implemented in AMP because it only relates to IE11, a browser which now has a very small market share.
*
* comment-reply:
* Support for comment replies is provided by the AMP plugin.
*/
function _s_scripts() {
if ( _s_is_amp() ) {
return;
}

wp_enqueue_script( '_s-navigation', get_template_directory_uri() . '/js/navigation.js', array(), '20151215', true );

Expand Down
28 changes: 25 additions & 3 deletions header.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<html <?php language_attributes(); ?>>
<head>
<meta charset="<?php bloginfo( 'charset' ); ?>">
<meta name="viewport" content="width=device-width, initial-scale=1">
westonruter marked this conversation as resolved.
Show resolved Hide resolved
<meta name="viewport" content="width=device-width, initial-scale=1, minimum-scale=1">
<link rel="profile" href="https://gmpg.org/xfn/11">

<?php wp_head(); ?>
Expand Down Expand Up @@ -44,8 +44,30 @@
<?php endif; ?>
</div><!-- .site-branding -->

<nav id="site-navigation" class="main-navigation">
<button class="menu-toggle" aria-controls="primary-menu" aria-expanded="false"><?php esc_html_e( 'Primary Menu', '_s' ); ?></button>
<nav
id="site-navigation"
class="main-navigation"
<?php if ( _s_is_amp() ) : ?>
[class]=" siteNavigationMenuExpanded ? 'main-navigation toggled' : 'main-navigation' "
<?php endif; ?>
>
<?php if ( _s_is_amp() ) : ?>
<amp-state id="siteNavigationMenuExpanded">
<script type="application/json">false</script>
</amp-state>
<?php endif; ?>

<button
class="menu-toggle"
aria-controls="primary-menu"
aria-expanded="false"
<?php if ( _s_is_amp() ) : ?>
on="tap:AMP.setState( { siteNavigationMenuExpanded: ! siteNavigationMenuExpanded } )"
[aria-expanded]="siteNavigationMenuExpanded ? 'true' : 'false'"
<?php endif; ?>
>
<?php esc_html_e( 'Primary Menu', '_s' ); ?>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not figure out what the button is for. Is it for the mobile menu?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. If you resize the window to the mobile breakpoint then this button appears.

</button>
<?php
wp_nav_menu( array(
'theme_location' => 'menu-1',
Expand Down
4 changes: 2 additions & 2 deletions inc/custom-header.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ function _s_header_style() {
position: absolute;
clip: rect(1px, 1px, 1px, 1px);
}
<?php
// If the user has set a custom color for the text use that.
<?php
else :
// If the user has set a custom color for the text use that.
?>
.site-title a,
.site-description {
Expand Down
2 changes: 1 addition & 1 deletion inc/woocommerce.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ function _s_woocommerce_wrapper_before() {
* @return void
*/
function _s_woocommerce_wrapper_after() {
?>
?>
</main><!-- #main -->
</div><!-- #primary -->
<?php
Expand Down
1 change: 1 addition & 0 deletions phpcs.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,5 @@
</properties>
</rule>

<exclude-pattern>rtl.css</exclude-pattern>
</ruleset>
13 changes: 13 additions & 0 deletions sass/navigation/_menus.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,16 @@
}

li {
/* The focus class is added by navigation.js */
&:hover > ul,
&.focus > ul {
left: 100%;
}

/* The :focus-within is for non-JS contexts and AMP (all browsers except IE/Edge support) */
&:focus-within > ul {
left: 100%;
}
}

a {
Expand All @@ -43,16 +49,23 @@
}
}

/* The focus class is added by navigation.js */
li:hover > ul,
li.focus > ul {
left: auto;
}

/* The :focus-within is for non-JS contexts and AMP (all browsers except IE/Edge support) */
li:focus-within > ul {
left: auto;
}
}

li {
float: left;
position: relative;

/* The focus class is added by navigation.js */
&:hover > a,
&.focus > a {
}
Expand Down
23 changes: 18 additions & 5 deletions style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,20 +664,33 @@ a:hover, a:active {
top: 0;
}

/* The focus class is added by navigation.js */
.main-navigation ul ul li:hover > ul,
.main-navigation ul ul li.focus > ul {
.main-navigation ul ul li.focus > ul
{
left: 100%;
}

.main-navigation ul ul a {
width: 200px;
}

.main-navigation ul li:hover > ul,
.main-navigation ul li.focus > ul {
left: auto;
}

/*
* Alternatives to focus class for supporting browsers (all but IE/Edge) for no-JS context (e.g. AMP)
* See https://caniuse.com/#feat=css-focus-within
*/
.main-navigation ul ul li:focus-within > ul {
left: 100%;
}
.main-navigation ul li:focus-within > ul {
left: auto;
}

.main-navigation ul ul a {
width: 200px;
}

.main-navigation li {
float: left;
position: relative;
Expand Down