Skip to content
This repository has been archived by the owner on Dec 1, 2019. It is now read-only.

Update the site header and menus to match new structure #246

Closed
andersnoren opened this issue Sep 13, 2019 · 7 comments
Closed

Update the site header and menus to match new structure #246

andersnoren opened this issue Sep 13, 2019 · 7 comments

Comments

@andersnoren
Copy link
Contributor

andersnoren commented Sep 13, 2019

This issue details the implementation of the new header structure, based on the discussions in the following issues: #164 #74. Thanks to everyone who has contributed to the discussion. I’ve posted a summary of the reasoning behind the final structure in this issue: #164. It’s on the lengthy side, so I won’t repeat all of it here.

The new structure will require changes to the menu locations, the output of the menus and some surrounding elements, and the styling. I’ve split them up into three headings below.

NOTE: The new folder structure should be implemented before we start work on this. See #244

Menu locations

The following menu locations will be used:

  • primary Desktop Horizontal Menu
  • expanded Desktop Expanded Menu
  • mobile Mobile Menu
  • footer Footer Menu
  • social Social Menu

The ID’s have been selected to help WordPress map existing menus to their corresponding locations in Twenty Twenty when users activate the theme. More info on that in #164.

Output changes

header.php

  • The wp_nav_menu() call (and corresponding has_nav_menu() conditional) for the primary menu (previously shortcuts menu) needs to be updated with the new primary menu location name.
  • If the primary menu is the primary menu, we should include the wp_list_pages() fallback here instead of in the modal-menu.php.
  • The class names and selectors for the shortcuts menu should probably be changed to be called primary instead of shortcuts, for consistency.

template-parts/modal-menu.php

The styling for the mobile and expanded menu is shared, so they should have a shared class that can be used for that targeting. It’s currently using .main-menu as a top level selector for the menu styles.

If both the mobile and expanded locations are set, and they are set to two different menus, both should be output with two different wp_nav_menu calls in the modal menu. The mobile one should be displayed up to 1000px width, and the expanded one should be displayed from 1000px and up.

If both menu locations are set, but to the same menu, only one element should be output, to save on overall page size.

If the expanded menu isn’t set, it shouldn’t be output, and the menu toggle should be hidden on 1000px and up.

If the mobile menu isn’t set, we should first check if a menu is set to the primary location, and if it is, use that menu instead. If not, check if expanded location is set, and if it is, use that menu. If neither are set, use a wp_list_pages() fallback.

The wp_nav_menu() call for the social menu needs to be updated with the new social menu location.

The ”Close Menu” button needs to be added, and set to be displayed on 1000px and up. We can add a conditional to only output this when the expanded menu location is set, since the header menu toggle is used for the mobile menu. See the ”Expanded menu” heading in the ”Style changes” section of the issue for an image of the button.

footer.php

The wp_nav_menu() call for the footer menu needs to be updated with the new footer menu location, and the wp_nav_menu() call for the social menu needs to be updated with the new social menu location.

functions.php

The twentytwenty_add_sub_toggles_to_main_menu() function needs to be updated to match the new menu locations. Toggles need to be added to both the mobile location and the expanded location, and the menu location conditional for adding the chevron-down icon needs to be changed from shortcuts-menu to primary.

Ideally, we should also add a corresponding function for appending sub menu toggles to the output of the wp_list_pages() fallbacks in the mobile menu.

Style changes

Mobile menu

The wp_list_pages() fallback output needs to be styled to match the wp_nav_menu() output when a menu is set.

This is the design for the mobile menu:

mobile-menu

The social icons should have the accent color as their background color, and the top level menu items should be bold, have the accent color and a font size of 2rem.

Primary menu

The wp_list_pages() fallback output needs to be styled to match the wp_nav_menu() output when a menu is set.

Expanded menu

This is the design for the expanded menu, with both a expanded menu and social menu set:

Expanded Menu

The width of the section needs to be changed to 50rem, and overall padding needs to be adjusted. The new ”Close Menu” button also needs styling. The styles carried over from the changes in the ”Mobile menu” section should cover most of the rest (font-weight, colors).


Let me know if I have missed something, and I'll add it above.

@carolinan
Copy link
Contributor

In #247 I have done the basic name changes but not added the logic for showing the correct menu (mobile or expanded) in modal-menu.php.

@nielslange
Copy link
Collaborator

Happy to implement the open issues, @carolinan.

@joyously
Copy link

Your design for Mobile menu shows the chevron pointing up when closed and when open, but also pointing down when closed. And then the Expanded menu shows the chevron pointing down when closed.
Could you clarify if the chevron is a state indicator or a label for the action you get when clicking?
I assume that the two menus should use them the same way.

@acosmin
Copy link
Contributor

acosmin commented Sep 13, 2019

@joyously I think those are just some mockups, the chevron is pointing the right way in the coded version.

@andersnoren
Copy link
Contributor Author

Yep, that’s me making a mess of things in Figma. It should always point down when it’s closed, and point up when it’s open.

andersnoren pushed a commit that referenced this issue Sep 15, 2019
* Update Menu names

Partial fix for #246
Fixes #73 (nav)

* Update header.php

Updated spacing to pass PHPCS

* fix PHPCS issues

* Trying to fix tabbing again
@andersnoren
Copy link
Contributor Author

PSA that I'll be working on this today, building on the updates in #247.

@andersnoren
Copy link
Contributor Author

Fixed by #282.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants