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

[amp-story-player] Player should allow scrolling when possible #28009

Closed
amanintech opened this issue Apr 24, 2020 · 7 comments · Fixed by #31051
Closed

[amp-story-player] Player should allow scrolling when possible #28009

amanintech opened this issue Apr 24, 2020 · 7 comments · Fixed by #31051

Comments

@amanintech
Copy link
Member

amanintech commented Apr 24, 2020

I saw a use case where amp story can be embedded inside another site using iframe. Now When the user comes to this section he is totally stuck and can't scroll or move. This is because the AMP story tap suggestion restrict him from scrolling

Is there a way to remove tap suggestions when the user accidentally tries to scroll on the amp story?

Example URL https://twimbit.com/company/ on Android chrome browser

@Enriqe
Copy link
Contributor

Enriqe commented Apr 24, 2020

Hey, thanks for the report and example!

I'm trying to think of a good approach to solve for this use case. Perhaps allowing vertical scroll events via a url param that the iframe host sets. There are some complications with this, though: e.g. when there is a page-attachment aka "swipe up". In this case, should a swipe up scroll, or open the page-attachment?

Some iframe-based components in the wild solve this by introducing two-finger swipes. e.g. if you want to interact with a Google Maps embed in a website, you have to use two-fingers. Otherwise it defaults to scrolling. We could think of doing something similar for swipe-up, and add an education banner that prompts the user to use two fingers to interact with the swipe up. Just an initial thought. @gmajoulet do you have any thoughts around this?

/cc @ampproject/wg-stories

@gmajoulet
Copy link
Contributor

There are a lot of precedents for this kind of issue, brought to us by years of carousel components trends :))

The biggest issue I see on the page you linked to, is that on swipe up nothing interesting happens: the page does not scroll, and the story does nothing interesting (the tap back tap next education is irrelevant to what the user is trying to do).

I believe the Story should behave just like a scrollable div:

  • If there is something to scroll within the Story (such as the swipe up page attachment), it performs that action over the webpage scroll
  • If there is nothing to scroll within the Story, it lets the touch events bubble so the webpage can scroll

I'm not 100% sure how to implement this but we can start looking into it. We have various viewer messaging things to handle touch events or check if the current page has a page attachment.

@amanintech
Copy link
Member Author

amanintech commented Apr 28, 2020

Awesome,
I wonder if we can offer a parameter property like scroll=true that lets the user decide what they want to do with the scroll. Another way around is to detect the presence of scrollY and disable the Tap suggestion.
Let me know if I can contribute in any way

@Enriqe Enriqe changed the title Tap Suggestion on AMP story doesn't let the user scroll on mobile [amp-story-player] Player should allow scrolling when possible May 21, 2020
@Enriqe Enriqe added this to To do in wg-stories Player via automation Jul 16, 2020
@Enriqe Enriqe moved this from To do to Bugs in wg-stories Player Oct 2, 2020
@dreamofabear
Copy link

dreamofabear commented Oct 22, 2020

FYI we've received a couple reports of this for the WordPress editor so far:

My 2c:

  1. P1: Fix the scroll touch consumption bug.
  2. P2 FR: Support an initial inactive/preview state. Activating the player expands the story to full-screen mode (amp-story-player: "Expand to fullscreen" button #30377), similar to a YouTube embed. This could help prevent users from interacting with stories in the awkward "windowed mode".

Not sure if this was already suggested/discussed. /cc @raovs @hongcatlover

@Enriqe
Copy link
Contributor

Enriqe commented Oct 23, 2020

I can look into this soon. I don't think the "touch to full-screen mode" is a good default though, since we don't really have context on where the player is embedded or what it would take to make it actually go "full screen".

I do think that #28009 (comment) is a good start though. WDYT @raovs @hongcatlover ?

@dreamofabear
Copy link

It would have to be publisher-configurable. But yea, let's fix the scrolling issue first.

@raovs
Copy link
Contributor

raovs commented Oct 23, 2020

+1 to Publisher configured fullscreen / lightbox mode (pull @hongcatlover in if you need UX)

If the publisher wants the content to be consumed within the embed we can do a few things:

  1. As is being discussed, don't consume interactions that don't apply to the Story
  2. On first tap, show an X in the top to exit interactions with the Story (another tap would reopen engagement with the Story)

@Enriqe Enriqe added this to To do in wg-stories Sprint via automation Nov 5, 2020
@Enriqe Enriqe moved this from Bugs to In Progress in wg-stories Player Nov 18, 2020
@Enriqe Enriqe moved this from To do to In progress in wg-stories Sprint Nov 18, 2020
wg-stories Player automation moved this from In Progress to Done Nov 19, 2020
wg-stories Sprint automation moved this from In progress to Done Nov 19, 2020
@mszylkowski mszylkowski removed this from Done in wg-stories Sprint Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

5 participants