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

WIP: Adds mobile menu using AMP #120

Closed
wants to merge 10 commits into from
Closed

WIP: Adds mobile menu using AMP #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0bdba4b
Add 'hamburger' menu icon to SVG icon set.
laurelfulford Jul 24, 2019
df81536
Add initial pass of new files for mobile navigation and the mobile si…
laurelfulford Jul 24, 2019
deaa3fa
Add sidebar menu toggle, and menu sidebar template part.
laurelfulford Jul 24, 2019
8d79833
Make social menu styles more general so they can be picked up by the …
laurelfulford Jul 24, 2019
5c12669
Improving mobile menu styles.
laurelfulford Jul 24, 2019
095f1a3
Updating button class.
laurelfulford Jul 24, 2019
5e4711e
Update tertiary nav classes.
laurelfulford Jul 24, 2019
87eed68
Updating tertiary nav class.
laurelfulford Jul 24, 2019
27012fb
Adjusting mobile menu styles; updating existing classes so they inher…
laurelfulford Jul 24, 2019
e66f0a2
Reverting changes to priority menu functions; they were removed in a …
laurelfulford Jul 24, 2019
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
5 changes: 5 additions & 0 deletions classes/class-newspack-svg-icons.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,11 @@ public static function get_social_link_svg( $uri, $size ) {
<path d="M15.5 14h-.79l-.28-.27C15.41 12.59 16 11.11 16 9.5 16 5.91 13.09 3 9.5 3S3 5.91 3 9.5 5.91 16 9.5 16c1.61 0 3.09-.59 4.23-1.57l.27.28v.79l5 4.99L20.49 19l-4.99-5zm-6 0C7.01 14 5 11.99 5 9.5S7.01 5 9.5 5 14 7.01 14 9.5 11.99 14 9.5 14z"/>
<path d="M0 0h24v24H0z" fill="none"/>
</svg>',
'menu' => /* material-design - menu */ '
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24">
<path d="M0 0h24v24H0z" fill="none"/>
<path d="M3 18h18v-2H3v2zm0-5h18v-2H3v2zm0-7v2h18V6H3z"/>
</svg>',

'close' => /* material-design - close */ '
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24">
Expand Down
17 changes: 13 additions & 4 deletions header.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@
</head>

<body <?php body_class(); ?>>

<?php get_template_part( 'template-parts/header/mobile', 'sidebar' ); ?>

<div id="page" class="site">
<a class="skip-link screen-reader-text" href="#content"><?php _e( 'Skip to content', 'newspack' ); ?></a>

<header id="masthead" class="site-header hide-header-search" [class]="searchVisible ? 'show-header-search site-header ' : 'hide-header-search site-header'">

<div class="top-header-contain">
<div class="top-header-contain desktop-navigation">
<div class="wrapper">
<?php if ( has_nav_menu( 'secondary-menu' ) ) : ?>
<nav class="secondary-menu" aria-label="<?php esc_attr_e( 'Secondary Menu', 'newspack' ); ?>">
Expand Down Expand Up @@ -65,23 +68,29 @@
<?php get_template_part( 'template-parts/header/site', 'branding' ); ?>

<?php if ( has_nav_menu( 'tertiary-menu' ) ) : ?>
<nav class="tertiary-menu" aria-label="<?php esc_attr_e( 'Tertiary Menu', 'newspack' ); ?>">
<nav class="tertiary-menu desktop-navigation" aria-label="<?php esc_attr_e( 'Tertiary Menu', 'newspack' ); ?>">
<?php
wp_nav_menu(
array(
'theme_location' => 'tertiary-menu',
'menu_class' => 'tertiary-menu',
'items_wrap' => '<ul id="%1$s" class="%2$s">%3$s</ul>',
'depth' => 1,
)
);
?>
</nav>
<?php endif; ?>

<?php if ( function_exists( 'is_amp_endpoint' ) && is_amp_endpoint() ) : ?>
<button class="mobile-menu-toggle" on='tap:mobile-sidebar.toggle'>
<?php echo wp_kses( newspack_get_icon_svg( 'menu', 20 ), newspack_sanitize_svgs() ); ?>
<?php esc_html_e( 'Menu', 'newspack' ); ?>
</button>
<?php endif; ?>
</div><!-- .wrapper -->
</div><!-- .middle-header-contain -->

<div class="bottom-header-contain">
<div class="bottom-header-contain desktop-navigation">
<div class="wrapper">
<?php if ( has_nav_menu( 'primary-menu' ) ) : ?>
<nav id="site-navigation" class="main-navigation" aria-label="<?php esc_attr_e( 'Top Menu', 'newspack' ); ?>">
Expand Down
71 changes: 71 additions & 0 deletions inc/template-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,42 @@ function newspack_get_discussion_data() {
return $discussion;
}

/**
* Add an extra menu to our nav for our priority+ navigation to use
*
* @param object $nav_menu Nav menu.
* @param object $args Nav menu args.
* @return string More link for hidden menu items.
*/
function newspack_add_ellipses_to_nav( $nav_menu, $args ) {

if ( 'primary-menu' === $args->theme_location ) :

$nav_menu .= '<div class="main-menu-more">';
Copy link
Contributor

Choose a reason for hiding this comment

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

This div is causing a validation error here. The AMP plugin is reporting the element as being invalid here because it is violating this constraint in the amp-sidebar > nav tag spec:

  child_tags: {
    mandatory_num_child_tags: 1
    child_tag_name_oneof: "OL"
    child_tag_name_oneof: "UL"
  }

Nevertheless, even though the AMP plugin is reporting it as a validation error, it is not removing it. Also, the markup is not causing validation errors in the AMP Validator. So I'm confused.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what you have is correct here, and there is a bug with the AMP plugin. Namely, because the parent nav does not have a toolbar attribute, this constraint should not be applying.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've opened a fix: ampproject/amp-wp#2926

$nav_menu .= '<ul class="main-menu">';
$nav_menu .= '<li class="menu-item menu-item-has-children">';
$nav_menu .= '<button class="submenu-expand main-menu-more-toggle is-empty" tabindex="-1" aria-label="More" aria-haspopup="true" aria-expanded="false">';
$nav_menu .= '<span class="screen-reader-text">' . esc_html__( 'More', 'newspack' ) . '</span>';
$nav_menu .= newspack_get_icon_svg( 'arrow_drop_down_ellipsis' );
$nav_menu .= '</button>';
$nav_menu .= '<ul class="sub-menu hidden-links">';
$nav_menu .= '<li id="menu-item--1" class="mobile-parent-nav-menu-item menu-item--1">';
$nav_menu .= '<button class="menu-item-link-return">';
$nav_menu .= newspack_get_icon_svg( 'chevron_left' );
$nav_menu .= esc_html__( 'Back', 'newspack' );
$nav_menu .= '</button>';
$nav_menu .= '</li>';
$nav_menu .= '</ul>';
$nav_menu .= '</li>';
$nav_menu .= '</ul>';
$nav_menu .= '</div>';

endif;

return $nav_menu;
}
add_filter( 'wp_nav_menu', 'newspack_add_ellipses_to_nav', 10, 2 );

/**
* WCAG 2.0 Attributes for Dropdown Menus
*
Expand Down Expand Up @@ -302,6 +338,41 @@ function newspack_add_dropdown_icons( $output, $item, $depth, $args ) {
}
add_filter( 'walker_nav_menu_start_el', 'newspack_add_dropdown_icons', 10, 4 );

/**
* Create a nav menu item to be displayed on mobile to navigate from submenu back to the parent.
*
* This duplicates each parent nav menu item and makes it the first child of itself.
*
* @param array $sorted_menu_items Sorted nav menu items.
* @param object $args Nav menu args.
* @return array Amended nav menu items.
*/
function newspack_add_mobile_parent_nav_menu_items( $sorted_menu_items, $args ) {
static $pseudo_id = 0;
if ( ! isset( $args->theme_location ) || 'primary-menu' !== $args->theme_location ) {
return $sorted_menu_items;
}

$amended_menu_items = array();
foreach ( $sorted_menu_items as $nav_menu_item ) {
$amended_menu_items[] = $nav_menu_item;
if ( in_array( 'menu-item-has-children', $nav_menu_item->classes, true ) ) {
$parent_menu_item = clone $nav_menu_item;
$parent_menu_item->original_id = $nav_menu_item->ID;
$parent_menu_item->ID = --$pseudo_id;
$parent_menu_item->db_id = $parent_menu_item->ID;
$parent_menu_item->object_id = $parent_menu_item->ID;
$parent_menu_item->classes = array( 'mobile-parent-nav-menu-item' );
$parent_menu_item->menu_item_parent = $nav_menu_item->ID;

$amended_menu_items[] = $parent_menu_item;
}
}

return $amended_menu_items;
}
add_filter( 'wp_nav_menu_objects', 'newspack_add_mobile_parent_nav_menu_items', 10, 2 );

/**
* Adjust a hexidecimal colour value to lighten or darken it.
*
Expand Down
72 changes: 72 additions & 0 deletions sass/navigation/_menu-mobile-navigation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
.mobile-menu-toggle {
background-color: transparent;
color: $color__text-main;
padding: 0;

&:hover,
&:focus {
background-color: transparent;
}

.svg-icon {
position: relative;
top: 4px;
}

@include media(tablet) {
display: none;
}
}

.desktop-navigation {
display: none;
}

@include media( tablet ) {
.desktop-navigation {
display: block;

&.tertiary-menu {
display: flex;
}
}
}

#mobile-sidebar {
background-color: $color__primary;
font-size: $font__size-sm;
padding: $size__spacing-unit;

& > * {
margin-bottom: $size__spacing-unit;
}

.mobile-menu-toggle {
color: #fff;
font-size: 1em;
margin: 0 0 $size__spacing-unit;
padding: 0;
}

ul {
font-weight: 700;
list-style: none;
margin: 0;
padding: 0;

ul {
font-weight: 400;
margin-left: $size__spacing-unit;
}
}

a {
color: #fff;
display: inline-block;
padding: #{ 0.25 * $size__spacing-unit } 0;
}

.submenu-expand {
display: none;
}
}
65 changes: 30 additions & 35 deletions sass/navigation/_menu-social-navigation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,44 @@
/* Social menu */

.social-navigation {
.social-links-menu {
align-items: center;
display: flex;
margin: 0;
padding: 0;

ul {
display: flex;
margin: 0;
padding: 0;

li {
list-style: none;
li {
list-style: none;

&:nth-child(n+2) {
margin-left: 0.5em;
}
&:nth-child(n+2) {
margin-left: 0.5em;
}

a {
border-bottom: 1px solid transparent;
display: block;
color: inherit;
margin-bottom: -1px;
transition: opacity $link_transition ease-in-out;
a {
border-bottom: 1px solid transparent;
display: block;
color: inherit;
margin-bottom: -1px;
transition: opacity $link_transition ease-in-out;

&:hover,
&:active {
opacity: 0.6;
}
&:hover,
&:active {
opacity: 0.6;
}

&:focus {
opacity: 1;
border-bottom: 1px solid $color__text-main;
}
&:focus {
opacity: 1;
border-bottom: 1px solid $color__text-main;
}

svg {
display: block;
width: 24px;
height: 24px;
svg {
display: block;
width: 24px;
height: 24px;

// Prevent icons from jumping in Safari using hardware acceleration.
transform: translateZ(0);
// Prevent icons from jumping in Safari using hardware acceleration.
transform: translateZ(0);

&#ui-icon-link {
transform: rotate(-45deg);
}
&#ui-icon-link {
transform: rotate(-45deg);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions sass/navigation/_menu-tertiary-navigation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** === Tertiary menu === */

nav.tertiary-menu {
.tertiary-menu {
align-items: center;
display: flex;
font-family: $font__heading;
Expand Down Expand Up @@ -44,7 +44,7 @@ nav.tertiary-menu {
}

body:not(.header-solid-background) {
nav.tertiary-menu {
.tertiary-menu {
a {
background-color: lighten($color__text-light, 45%);
color: $color__text-main;
Expand Down
1 change: 1 addition & 0 deletions sass/navigation/_navigation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
@import "menu-tertiary-navigation";
@import "menu-social-navigation";
@import "menu-footer-navigation";
@import "menu-mobile-navigation";

/*--------------------------------------------------------------
## Next / Previous
Expand Down
15 changes: 11 additions & 4 deletions sass/site/header/_site-header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,15 @@
// Site description
.site-description {
color: $color__text-light;
display: none;
font-weight: normal;
font-size: $font__size-sm;
font-style: italic;
margin: 7px 0 0;

@include media( mobile ) {
display: block;
}
}

.hide-site-tagline .site-description {
Expand All @@ -79,10 +84,12 @@

// Middle bar
.middle-header-contain {
align-items: center;

.wrapper {
padding: #{ 2 * $size__spacing-unit } 0;
align-items: center;
padding: $size__spacing-unit 0;
@include media( mobile ) {
padding: #{ 2 * $size__spacing-unit } 0;
}
}
}

Expand Down Expand Up @@ -166,7 +173,7 @@ body:not(.header-center-logo) {
}

.social-navigation,
nav.tertiary-menu {
.tertiary-menu {
justify-content: center;
width: 100%;

Expand Down
1 change: 1 addition & 0 deletions sass/typography/_headings.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
.site-info,
#cancel-comment-reply-link,
.use-header-font,
#mobile-sidebar,
h1,
h2,
h3,
Expand Down
Loading