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

Intent-to-Deprecate: History Manipulation of Modal Components #25080

Closed
cathyxz opened this issue Oct 16, 2019 · 26 comments
Closed

Intent-to-Deprecate: History Manipulation of Modal Components #25080

cathyxz opened this issue Oct 16, 2019 · 26 comments
Assignees
Labels
INTENT TO DEPRECATE Proposes deprecating an existing AMP feature. WG: components

Comments

@cathyxz
Copy link
Contributor

cathyxz commented Oct 16, 2019

Summary

We currently have features in components like amp-sidebar and amp-lightbox-gallery that add operations of open / close modal to history state. We intend to deprecate them by introducing an experiment and gradually ramping this up.

Motivation

Due to implementation of features like "swipe-to-go-back" on Safari, our current manipulation of history via components like amp-sidebar result in unintuitive end user experience. History manipulation has also been known to cause issues such as #6585, where link navigation is broken when history pop is happening at the same time. Example of weird UX: link, try opening sidebar and then swipe-to-go-back on iOS Safari (see gif below for actual behavior).

Impact on existing users

Users will no longer be able to open / close modals (e.g. sidebar, lightbox-gallery, lightbox) using the back button.

Alternative implementation suggestion for developers using AMP

This is open to discussion, we can consider preserving this functionality under an attribute, where the default is to have this behavior turned off instead of on.

Additional context

We intend to deprecate this feature by first introducing an experiment that turns off history manipulation for amp-sidebar and amp-lightbox-gallery. The intention is to gradually ramp up this experiment over time. Based on community feedback, either completely remove this feature, or to convert this behavior to either a default-on or default-off attribute.

/cc @ampproject/wg-approvers @ampproject/wg-ui-and-a11y

Current Behavior on Safari Mobile

Safari

Edits:

  • Updated issue description with demo link.
  • Updated issue description with gif (thank you @jridgewell).
@cathyxz cathyxz added INTENT TO DEPRECATE Proposes deprecating an existing AMP feature. WG: components labels Oct 16, 2019
@jridgewell
Copy link
Contributor

This'll make Bento muuuccchhh easier.

@cramforce
Copy link
Member

cramforce commented Oct 16, 2019 via email

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 16, 2019

amp-lightbox and it's modal usage, was specifically one component where this feature might still make sense (which is why I didn't list amp-lightbox in the deprecation list). amp-sidebar, since we animate it's open and close, was one situation in which swipe-to-go-back could make the sidebar open from the wrong side of the page, and that would result in an awkward UX. amp-lightbox-gallery, also has some pretty fancy transition animations by default. I can see how swipe-to-go-back triggering these animations would result in a strange UX.

@cathyxz cathyxz self-assigned this Oct 16, 2019
@cramforce
Copy link
Member

cramforce commented Oct 16, 2019 via email

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 16, 2019

Yup, one is https://amp.dev/documentation/examples/components/amp-sidebar/preview/?format=websites on iOS Safari. Opening the sidebar and then swipe-to-go-back results in going back to the same page, and then the sidebar animates close.

@cramforce
Copy link
Member

cramforce commented Oct 16, 2019 via email

@jridgewell
Copy link
Contributor

back swipe on iOS

@cramforce
Copy link
Member

cramforce commented Oct 16, 2019 via email

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 16, 2019

Yup. In iOS, the interaction of history with animated transitions looks super broken because the swipe gesture opens an overlay of the previous page (in the history API). It results in swiping to the same page, and then triggering an animated transition. It looks extra broken when the direction you swipe is the opposite of the direction that the sidebar closes to (sidebars can be configured to open from left or right). @kristoferbaxter and @sparhami voiced concerns that maintaining history state in modals for desktop wasn't a very common pattern either, but as a minimum this interaction doesn't feel right on Safari.

@dvoytenko
Copy link
Contributor

Following the native land, however, the back navigation is used very aggressively in sidebars, popups, and lightboxes. Is this just a technical problem on iOS? Can it be resolved by delayed history pop slightly?

@sparhami
Copy link

Following the native land, however, the back navigation is used very aggressively in sidebars, popups, and lightboxes. Is this just a technical problem on iOS? Can it be resolved by delayed history pop slightly?

The problem is in native, you don't have this persistent swipe from the edge behavior for dismissing things. For example, a lightbox has a swipe to dismiss, but that is a downwards or upwards swipe. For example, in the lightbox view for the iOS photos / Google Photos apps, a swipe from the left, even on the edge, navigates the carousel.

On iOS this gets a bit confusing. If you swipe away from the edge, it navigates the carousel. However, if you swipe near the edge, it will navigate back (the UI will look incorrect, the preview of what is being swiped back to looks like the lightbox open state), but then the lightbox closes. The preview image that iOS captured for the previous history state is not correct.

I think the sidebar is an interesting example. In in some native apps (e.g. Gmail on Android and iOS), swiping from the side of the screen will cause the sidebar to open as well as close. Also, swiping from right to left (in LTR languages) is what closes the sidebar, not a swipe from left to right (back navigation gesture in iOS).

I think the expectation on the web in iOS is that swiping from the edge will take you back to the previous page rather than closing a modal. I'm not sure native apps are the right model, and they often don't really have a consistent concept of different pages (with back/forwards) other than what an app itself implements. Some apps have left/right swipes navigate to different sections in some views. In other views, swiping left to right will close (aka go back) but then swiping right to left will not reopen (aka go forwards).

@cramforce
Copy link
Member

cramforce commented Oct 16, 2019 via email

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 16, 2019

Worth mentioning that we also recently implemented swipe-to-dismiss for sidebar itself (we have swipe to dismiss for lightbox-gallery already). So if you swipe on the sidebar (not from the edge), you can actually close the sidebar already.

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 17, 2019

I thought about this some more, and the "broken" part of this experience really comes from the interaction with iOS's overlay and our animations (this looks fairly broken in amp-lightbox too). Maybe if we do want the back button to pop local state, the ideal solution is just to remove transition animations for iOS. But that said, I don't know what's the current consensus on whether a back button should navigate to the last page, or push / pop local state. I would love data on that too.

@dvoytenko
Copy link
Contributor

I think we should approach this carefully...

iOS is always tricky. But one way to look at this: how do native components with overlays work. E.g. <select>, <input type=date>, window.confirm(), etc. I know that Android cheats in a way with these, but they strongly support back button for these elements. I believe iOS generally prefers "Done" button somewhere in the UX. But back button support for "cancel" is meaningful here, I believe. Nothing worth than to fill out a 20 field form just to click back on the 20th input hoping to close the date selector and the whole page navigate back.

Lightbox, however, is slightly different UX, imho - it often appears as a drill-down navigation and thus the back button makes sense from the navigation point of view.

@cramforce
Copy link
Member

For reference, here is Android
ezgif-1-8fe18914353c

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 21, 2019

Summarizing feedback so far:

  1. History manipulation is useful in context of forms, we should keep that.
  2. History manipulation is problematic in modal components in iOS (due to swipe-to-go-back), but not necessarily so in Android.
  3. The most broken part of this experience is how our transition animations interact with iOS's swipe-to-go-back overlay of the previous page.

Given that, here are few potential action items that we can vote on:

  1. Remove transition animations for back action (iOS-only / globally) on sidebar-only.
  2. Remove transition animations for back action (iOS-only / globally) on
    modal components (i.e. sidebar, lightbox, lightbox-gallery).
  3. Remove history manipulation for back action (iOS-only / globally) on sidebar-only.
  4. Remove history manipulation for back action (iOS-only / globally) on all modal components (i.e. sidebar, lightbox, lightbox-gallery).

I'd like to propose:

Removing history manipulation on sidebar and removing transition animations for lightbox-gallery (lightbox has no transition animations I believe) for iOS only.

How do folks feel about launching an experiment (or a separate experiment per component) for that?

@jridgewell
Copy link
Contributor

History manipulation is useful in context of forms, we should keep that.

What's the use case here?

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 21, 2019

Maybe I'm misinterpreting @dvoytenko 's comment here, but my understanding is that we should allow people to navigate back to a previous field via back button in popup forms. For example, if we're in a lightbox containing a form, we should allow the back button to re-open a closed lightbox in case folks filled it out and accidentally tapped cancel--in this case, we should preserve the form content.

I think this is a legit use case. Though I wonder if mobile operating systems are going to be providing more and more native components for this kind of use case (context: Apple's view section under Human Interface Guidelines ). I also think sidebar might not be applicable to this use case (I don't think sidebar is meant to be used for containing long forms).

@dvoytenko
Copy link
Contributor

Yeah, I think I didn't explain myself very well. My use case was the popup-based controls such as a fancy date/date-range/etc input. The overlaying popups, imho, should be cancelable via back button. Thus I think we have 3 use cases:

  1. Popup overlays for menus or inputs.
  2. Lightboxes and dialogs
  3. Navigational tools such as sidebar

I know we did some polling around, but nothing that answers a specific question: The back button exists and clearly visible in iOS and Android most of the time -> in each of these use cases what's a user's expectation when clicking back button?

I think on Android things are a bit clearer since back button is shared with the platform in general and hence the native expectations are more naturally applied to the Web case.

I understand that on iOS we have some technical issues as well. But that's not uncommon. I think one of the first bugs filed on amp-carousel was a request to cancel iOS's back navigation when a user swipes on the carousel. And this was even more compounded given that Search on iOS swipes between documents. But somehow we navigated between all these cases.

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 24, 2019

Maybe we can do this. I'd like to launch an experiment to disable history manipulation on sidebar for iOS-only due to #25080 (comment). fix the sidebar glitch in iOS by either disabling history, or disabling the animation:
https://amp-sidebar-no-history.firebaseapp.com/
https://amp-sidebar-no-anim.firebaseapp.com/
I'll file a separate issue to track that as ab iOS bug, and leave this issue for a longer history manipulation discussion.

For the remainder of the use cases, it may be worthwhile to compile a list of use cases to answer @dvoytenko 's question of what do users expect when clicking the back button, but also evaluate how broken these experiences are when "back" is done via something like a swipe gesture. I can do that and paste a spreadsheet back here.

In short, I agree that in popup overlays for menus or inputs, we should maintain history so that a back button can take us back to our previously entered input. I think lightboxes and dialogs are a gray area, and maybe the problems and solutions are iOS specific, and mostly about how our current transition animations interact with iOS's gestures.

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 28, 2019

For reference, here's what they look like respectively:

Sidebar with animation disabled (iOS)

giphy

Sidebar experiment with history disabled (iOS)

(I think the gif was sped up)
200w_d

@dvoytenko
Copy link
Contributor

@cathyxz I'd really rather see us first trying to work around the animation part. I'm still very skeptical about disabling back button for sidebars or most of things that "pop up". I know first-hand how frustrating the History API be at times, but I still view back button support as a net win for the user.

@sparhami
Copy link

@dvoytenko The problems with history go beyond just the UI bug. They don't behave in a way that is consistent. For example, the following flow does not behave in a way that is consistent with history states:

  1. Open sidebar
  2. Click an item in sidebar, do a setState, then close the sidebar

-> There is now a history entry to go forward, but hitting forward does not re-open the sidebar

Since we added to the history stack, there is a next history entry for the sidebar state. The browser UI now shows the forward button (on iOS in the bottom bar, Chrome/Android has it in the 3 dot menu) as being available, and I can swipe forward (right edge to left) on iOS, which shows the sidebar as open in the preview state. When I go to the forward state, nothing happens.

Since the back/forwards cache does not work due to many analytics implementations using beforeUnload and unload listeners, you also run into the following bug on many (most?) pages:

  1. Open sidebar
  2. Click link (to another page)
  3. Hit back (original amp page loads, goes to the sidebar open history entry)
  4. Hit back again

-> Does nothing as far as the user can tell

Other flows are weird/broken too (e.g. on https://output.jsbin.com/sigupoy):

  1. Open sidebar
  2. Click an item that does pushState, closes sidebar

-> Bind changes the state, then closing the sidebar quickly reverts it back after a flash

  1. Open sidebar
  2. Click an item that closes sidebar, does pushState

-> Bind changes the state, then closing the sidebar quickly reverts it back after a flash

Your can setState, but that isn't correct either, since hitting back will leave the page rather that going back to the state prior to opening the sidebar and clicking the item. The way mobile apps behave is that hitting back acts as if you closed the sidebar then pushed a new state.

@dvoytenko
Copy link
Contributor

@sparhami These points still tell me that History API is pretty difficult to work with in its current state. But we are doing what we can do support the most obvious user flows. I definitely want to be realistic about it, but I'd also like to make sure we do all we can to support this. As for the specifics: it looks like we have two issues: forward button and push/pop races.

Forward button: I don't think this is much of an issue, tbh. In Android it's buried pretty deep. On iOS it's visible, but I'm not sure people give it a lot of attention. Unfortunately, I've been trained on the Web to almost never trust the forward button except for simplest cases. And part of the issue is with the push/pop race.

Push/pop race: ideally any out-of-sidebar action would wait for the sidebar to be closed and history popped. IIRC we've run into numerous issues with this given inconsistencies in how History API was implemented:

  1. history.go(-1) is weirdly synchronous-by-not-really. We do have code, I believe, to make it properly async, but not clear if it works in all cases. But I'm still not clear how often we have an issue with user gesture for sidebar actions.
  2. Out-of-page navigation on iOS specifically was navigating back due to the race between go(-1) and location.assign(url). I think we had issues even with our async go() hacks. I can't find the WebKit bug on this, but is it possible this has been fixed since then? Have we tried this recently?

Click an item that does pushState, closes sidebar

But how do you see this should work? Per API reading this is a bit confusing sequence.

Bind changes the state, then closing the sidebar quickly reverts it back after a flash

The sidebar comes back quickly? Or does the bind state reverts?

@cathyxz
Copy link
Contributor Author

cathyxz commented Nov 15, 2019

I think the conclusion of the design review was that we can solve the sidebar issue by disabling animation, and that we should do some UX research more broadly on the subject of history state in modal components across various platforms, since there is currently no clear consensus on that front. Looping in Design will also be helpful here. Let's table this discussion for now and revisit in the future.

@cathyxz cathyxz closed this as completed Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTENT TO DEPRECATE Proposes deprecating an existing AMP feature. WG: components
Projects
None yet
Development

No branches or pull requests

5 participants