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

touch devices: open submenus on the desktop horizontal menu #949

Merged
merged 5 commits into from
Nov 5, 2019
Merged

touch devices: open submenus on the desktop horizontal menu #949

merged 5 commits into from
Nov 5, 2019

Conversation

carolinan
Copy link
Contributor

Hopefully Fixes #790

@carolinan carolinan added help wanted Extra attention is needed menu labels Nov 4, 2019
pattonwebz
pattonwebz approved these changes Nov 4, 2019
View reviewed changes
Copy link
Member

@pattonwebz pattonwebz left a comment

Choose a reason for hiding this comment

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

This works fine for me and corrects the problem detailed in the issue

@carolinan carolinan merged commit 27d80e4 into WordPress:master Nov 5, 2019
@carolinan carolinan deleted the menu branch November 5, 2019 17:23
@pierlon pierlon mentioned this pull request Nov 7, 2019
Closed
3 tasks
@westonruter
Copy link
Member

@carolinan This fix is not working for me. I'm getting a JS error:

image

@pattonwebz
Copy link
Member

@westonruter I'm afraid that I missed this when I merged :(

I will try and look at this as soon as I can but if you have any ideas for a fix then let me know.

westonruter added a commit that referenced this pull request Nov 7, 2019
@westonruter
Revert: touch devices: open submenus on the desktop horizontal menu (#…
92e0a3e 
…949)
@westonruter
Copy link
Member

@pattonwebz I'm opening a PR.

@pattonwebz
Copy link
Member

Thank you for this 🙌 ❤️

Also as an aside I know you have been working to make sure this theme is AMP compatible and if there are other JS changes required for that let me know as we still have a very small window for adjusting important aspects and being AMP ready is important in my view

@westonruter westonruter mentioned this pull request Nov 7, 2019
Merged
@westonruter
Copy link
Member

westonruter commented Nov 7, 2019 •
edited
Loading

@pattonwebz See #971.

Otherwise, the theme has been made fully AMP compatible via the AMP plugin thanks to great work by @schlessera: ampproject/amp-wp#3342, ampproject/amp-wp#3403, ampproject/amp-wp#3606, ampproject/amp-wp#3604, ampproject/amp-wp#3616, ampproject/amp-wp#3628, ampproject/amp-wp#3621, ampproject/amp-wp#3618, ampproject/amp-wp#3617, ampproject/amp-wp#3620, ampproject/amp-wp#3594, ampproject/amp-wp#3578, ampproject/amp-wp#3574, ampproject/amp-wp#3570, ampproject/amp-wp#3569, ampproject/amp-wp#3559

In lieu of the Twenty Twenty theme being AMP-compatible in and of itself, the AMP plugin shims the compatibility via its core theme sanitizer. Perhaps in the future new core themes could be developed leveraging AMP components (especially when they can be installed locally as node packages via Bento AMP). But in the meantime, it's not so bad to maintain AMP compatibility for the core themes externally since they don't really change over time.

pattonwebz pushed a commit that referenced this pull request Nov 10, 2019
@westonruter @pattonwebz
Fix ability to expand nav submenus on large-viewport touch interfaces (…
db82df5 
…#971)

* Revert: touch devices: open submenus on the desktop horizontal menu (#949)

* Allow tapping submenu expansion arrows on touch screens

Props @pierlon

* Add touch-enabled JS polyfill for IE11

* Use any-pointer:course media query instead of excluding dual-mode interfaces
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pattonwebz pattonwebz pattonwebz approved these changes

@ianbelanger79 ianbelanger79 Awaiting requested review from ianbelanger79

Assignees
No one assigned
Labels
help wanted Extra attention is needed menu
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to access a submenu item of the Desktop Horizontal Menu on tablet (>1000px)
3 participants
@carolinan @westonruter @pattonwebz