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.

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

Ampify responsive Navigation Block #6838

Merged
merged 14 commits into from
Jan 25, 2022

Conversation

delawski
Copy link
Collaborator

@delawski delawski commented Jan 14, 2022

Summary

Fixes #6319
Closes #6744

This PR adds AMP support to the new Responsive Navigation block.

The key differences to the another solution proposed in #6744 are:

  • The AMP implementation reuses most of the non-AMP version, most notably the CSS.
  • amp-bind is used for toggling the appropriate class names and ARIA attributes (it replaces the micromodal logic from the original implementation).
  • No amp-lightbox or amp-sidebar is used.
  • In AMP_Core_Block_Handler::ampify_navigation_block no DOM tree manipulation is performed (only plain strings replacements).

When testing, it's important to use a menu that has at least one submenu (preferably more). It's because the submenus have their own toggle logic, independent of the overlay menu setting.

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@delawski delawski added Enhancement New feature or improvement of an existing one Integration: Gutenberg labels Jan 18, 2022
@delawski delawski added this to the v2.2.1 milestone Jan 18, 2022
@delawski delawski marked this pull request as ready for review January 18, 2022 17:38
@github-actions
Copy link
Contributor

github-actions bot commented Jan 18, 2022

Plugin builds for 95a3405 are ready 🛎️!

Copy link
Collaborator

@dhaval-parekh dhaval-parekh left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

…319-ampify-wp-navigation-block

* 'develop' of github.com:ampproject/amp-wp: (26 commits)
  Fix indentation
  Add assertions that no SQL queries run when data deletion is disabled
  Add test specifically for transient deletion
  Harden term deletion logic for WP<4.4
  Add assertions for term deletion
  Flesh out tests specifically for post and term deletion
  Add test specific to delete_user_metadata
  Add test specifically for the delete_options function
  Remove deletion of non-option
  Add test assertions for terms table
  Use constant for taxonomy slug in test
  Add formatting for SQL statements
  Normalize config.allow-plugins in composer.json
  Update database queries to delete data during uninstalltion
  Revert changes from delete_user_metadata()
  Use delete queries instead of WP function to delete posts and term during uninstalltion
  Add more unit test cases for uninstall.php
  Render value as fallback instead of entire parent source object
  Fix logic inversion for sources.length
  Ensure `sources` is non-empty array
  ...
@westonruter westonruter merged commit 0bb7948 into develop Jan 25, 2022
@westonruter westonruter deleted the feature/6319-ampify-wp-navigation-block branch January 25, 2022 23:22
westonruter added a commit that referenced this pull request Jan 25, 2022
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. Enhancement New feature or improvement of an existing one Integration: Gutenberg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for responsive navigation block in Gutenberg
3 participants