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

Feature request: Support for menu to display triggered by button tap #827

Closed
rudygalfi opened this issue Nov 1, 2015 · 36 comments
Closed
Assignees
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Needs UX Type: Feature Request

Comments

@rudygalfi
Copy link
Contributor

Menus can help drive circulation. This request covers the ability to show a menu once the user has clicked/tapped on a button presented on the page. The most common patterns for presenting a menu seem to be:

  • Upon pressing the menu button, the menu overlays the content on the page. There's some way to get rid of the menu either by clicking/tapping another button/link (likely in the same place as the original button) or by clicking/tapping outside the menu area.
  • Once the button is pressed, the menu slides in from the side and displaces the content (it slides as well). You can get rid of the menu either by clicking some button in the newly exposed menu area or by clicking in the content area (if some of it is still visible on-screen).
@dvoytenko
Copy link
Contributor

Rudy, wouldn't this be a viewer's function?

@rudygalfi
Copy link
Contributor Author

@dvoytenko I could see this behavior in the viewer, but that still raises the question of how that would work. Would there be some way of being able to define the menu content and, upon seeing that, a viewer provide a button that exposes the menu?

What I'm trying to reflect with this issue is some feedback I've heard from some publishers around enabling menus in some way. If you look at some mobile sites like http://mobile.nytimes.com/ or http://www.theguardian.com/mobile you can see examples, and that's the kind of pattern I have in mind here. These menus enable navigation to other areas like the Business or Sports section.

@dvoytenko
Copy link
Contributor

One approach would be to leave it completely up to the viewer implementation which is currently responsible for the header itself. That could work well in the publisher sites (e.g. Publisher.com viewer would show the Publisher.com's menu), but the question will be: should the menu be shown when the same story is displayed by, e.g. Google Search or by Twitter or others. The issue here might be that those apps may have their own menus.

@cramforce
Copy link
Member

There isn't always a viewer.
The most basic request here is to change layout based on whether there is.

I don't really understand the feature request, though. Whatever we do with menus must be responsive, so it does something reasonable on phone, tablets and desktop.

@rudygalfi
Copy link
Contributor Author

Let me know how I can clarify things. One thing I can say is that in creating this request I had the standalone (i.e. non-viewer) case in mind, so you should read everything that way: assuming no viewer. (I agree that it's interesting to consider menus in the context of a viewer as well.) I also agree that whatever we have should be responsive.

The current behavior we see is that publishers are providing links to other areas of their site (Sports section, Business section) through a statically defined list that's included in the page. You can't have any interactivity, so there's no way to press a button to be able to see the menu. This request captures some component that would allow this.

@mjsarfatti
Copy link

+1
I think the classic hamburger menu is seen more often than carousels or lightboxes, even though I see difficulties in finding one common way of implementing it (variations on the structure of the menu list are infinite, how would you manage submenus and sub-submenus?)

@maxhartshorn
Copy link

Yes this would be a very useful feature to have. I expect a user reading an article from any news site would want to have some way of navigating to other sections from the page they're in.

@heatloss
Copy link

I would highly urge that the HTML5 [details] and [summary] tags be supported, as these would allow for dynamic menus and accordion widgets without the need for any custom JavaScript.

@stephanfowler
Copy link

Menus on our regular site are geo-dependent. In the context of a single cached AMP article, this means the menu would have to be loaded following an interaction, which could make a call to our site and receive a geo-varied response.

So our requirement for menus is: an element (eg a "burger"), that when clicked, unhides a container and executes any AMP markup within it (such as amp-list etc.)

That only needs something like an Expando component, rather than anything specific to menus. Menus themselves are going to be too hard to generalise, imho.

@samthurman
Copy link

+1
We show share button menus with an initial tap, this feature would allow us to do that. Very much wanted!

@dkolba
Copy link

dkolba commented Feb 6, 2016

What's the ETA of Milestone M2?

@rudygalfi
Copy link
Contributor Author

@dkolba Roughly March/April. We have a lot in M2 currently, though, so some stuff may get further pushed out.

We're starting to make progress on menu-like functionality currently, however. cc @ericlindley-g

@steffenweber
Copy link
Contributor

Just a few thoughts: In HTML5 there is a hidden attribute that can be applied to any element. AMP should toggle (add/remove) this attribute to hide/show content.

Hidden state:

<button aria-controls="mypanel">Toggle</button>
<div id="mypanel" hidden>Text Text Text ...</div>

Expanded state:

<button aria-controls="mypanel" aria-expanded="true">Toggle</button>
<div id="mypanel">Text Text Text ...</div>

@gandmexpress
Copy link

Hi there, I'm brand new here and to AMP-html... actually I just founded today... but I just can't understand (after hours of looking into amp-html) why there would be no menu/nav options.. can someone please shed some light on this? So is really just to build single webpages... what's the purpose if you can't jump to a new page or have a navigation menu to help discover more of the owners site? Thank you

@Nemo64
Copy link
Contributor

Nemo64 commented Feb 18, 2016

@gandmexpress I'm not an amp dev but i think the reason is simple. Amp is still in its imfancy. A month ago it wasn't even possible to include analytics on a page.

@gandmexpress
Copy link

@Nemo64 makes sense... thank you.

@rudygalfi
Copy link
Contributor Author

Yes, as @Nemo64 said, AMP is still quite young. We've come a long way and we've built features in priority order based on feedback from publishers and content providers. (We also appreciate contributions to starting with an intent to implement and design overview to help solve the problems you're most interested in.) Menus are on our near-term roadmap. Another factor playing into the prioritization decision is that:
(a) you can build nav, just not necessarily with all of the interactive features you may like; and
(b) we've seen creative attempts to build menus using existing components like AMP lightbox, and while we think a more proper solution is needed, folks have made progress that way.

@gandmexpress
Copy link

@rudygalfi Rudy, thank you. I appreciate you taking the time to reply. I will look into alternatives mentioned in the mean time. I really like the idea/concept of what AMP is and will continue to bring the internet (specifically mobile). Thx again

@jpettitt
Copy link
Contributor

jpettitt commented Mar 2, 2016

You can achieve the same results (constrained to one open section) with :focus CSS selectors. I stole borrowed the idea from the Washington Post, they are doing 'hamburger' menus that way.

See https://www.washingtonpost.com/amphtml/politics/clinton-trump-victories-foreshadow-nasty-contentious-fall-campaign/2016/03/02/106c6dd6-e0ae-11e5-8d98-4b3d9215ade1_story.html

It's a bit tricky to get right and you need to put a tabindex on the elements or it won't work and you may need to add :active if you don't want things to disappear when links are clicked. Overall it seems to work pretty well. We're using it for footer menus and hamburger menus.

CSS

#accordion .footer-menu {
    display: none;
}
#accordion .heading:focus ~ .footer-menu {
    display: block;
}

html

<div id="accordion">
        <section class="panel">
            <a tabindex="99999" class="heading">Stuff</a>
            <ul class=" footer-menu">
                <li><a href="/">A link</a></li>
                <li><a href="/">a.n.other link</a></li>
            </ul>
        </section>

        <section class="panel">
            <a tabindex="99999" class="heading">Really interesting stuff</a>
            <ul class=" footer-menu">
                <li><a href="/">Customer Service</a></li>
                <li><a href="/">About Us</a></li>
            </ul>
        </section>

        <section class="panel">
            <a tabindex="99999" class="heading">Social, Mobile &amp; More</a>
            <ul class=" footer-menu">
                <li><a href="/">Facebook</a></li>
                <li><a href="/">Twitter</a></li>
                <li><a href="/">Mobile</a></li>
            </ul>
        </section>
</div>

@Nemo64
Copy link
Contributor

Nemo64 commented Mar 3, 2016

@jpettitt a nice idea. If we want workarounds I might as well just add the :target selector. It would not only allow for a collapsing menu but would also allow to use the back button on android to close it again. And it is also possible to scroll within the menu while still allowing other hash links to close it again. And it might work pretty well with accessibility when done right (put menu at the end of the page?).

I tried to construct something: https://jsfiddle.net/d6qr7qsh/1/

@rudygalfi rudygalfi modified the milestones: Backlog, M2 Mar 4, 2016
@dvoytenko
Copy link
Contributor

@jpettitt :focus most likely works with AMP JS on iOS Safari because we set cursor:pointer on html.

@steffenweber @Nemo64 :target is a very interesting idea. Although I'm leaning more and more toward a amp-sidebar or amp-menu element with flexible styling - there are just too many little bugs when working with position:fixed, scrolling and all the rest.

@steffenweber
Copy link
Contributor

Making :target work would have the nice side-effect of a free fallback for page requests with disabled JavaScript or JS errors.

@dvoytenko
Copy link
Contributor

@steffenweber @Nemo64 There's definitely a way to fix :target issue - #2456 is in review. That does have a nicer way to trigger the menu. However, that would leave with with fewer options to cancel the menu, e.g. by clicking outside or swiping. I think some JS intervention will be unavoidable, but I agree, the default behavior is of :target is good.

@rudygalfi
Copy link
Contributor Author

For folks interested in this issue, there's a pending pull request for an amp-sidebar component: #2461. Please take a look.

/cc @sriramkrish85 @ericlindley-g

@rudygalfi
Copy link
Contributor Author

Is #2461 tracking to land by tomorrow's release, or should we shift this into the next sprint?

@rudygalfi
Copy link
Contributor Author

Rolling this into next milestone.

@camelburrito camelburrito added the INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code label Apr 5, 2016
@camelburrito
Copy link
Contributor

Update:

#2461 - has been split into smaller PRs and all of them have landed.

#2788 , #2792, #2795 , #2811, #2812, #2813, #2823

There are a few more kinks that i am working on (#2859 - is one of them!)

Feel free to experiment and file bugs!

@camelburrito
Copy link
Contributor

@ericlindley-g This is ready for experimentation! and should be fully working now. There are a few validation fixes that needs to be done, then this should be fully functional (after a lot of testing!)

@ericlindley-g
Copy link
Contributor

Excellent! Is the plan to push to this week's canary?

/cc @edlea-g to take a look at the implementation

@camelburrito
Copy link
Contributor

@ericlindley-g yes (will still be an experiment)

@rudygalfi
Copy link
Contributor Author

Given there's additional open PRs (#827 (comment)) and assuming this is the meta-bug covering all of amp-sidebar, should we roll this over into the next milestone?

@camelburrito
Copy link
Contributor

@rudygalfi - all the PRs have been merged now. Sidebar is ready for experimenting (as mentioned above)

@rudygalfi
Copy link
Contributor Author

@sriramkrish85 Thanks, I'll close this out then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Needs UX Type: Feature Request
Projects
None yet
Development

No branches or pull requests