Intent-to-Remove: `amp-sidebar` 0.2

[wassgha]

Summary

amp-sidebar 0.2 (see #25049) was intended to be a breaking version bump of amp-sidebar with the following features:

1. Nested-menu support (which was then launched and integrated into 0.1)
2. Better animations (not implemented, but non-breaking so can be integrated into 0.1)
3. Uses accessibility best practices (non-breaking, partially implemented, will be brought into 0.1)
4. Solve Make <amp-sidebar> mask element compatible with Story #24827 by integrating the mask inside the amp-sidebar element (breaking, required 0.2)

However, from an offline discussion, it seems like #24827 is not worthy of a version bump since it's sole purpose is to provide a better solution for amp-story, however, amp-story will still need to continue to support amp-sidebar 0.1 anyways and the workaround that amp-story implemented will have to remain in the code. This makes the version bump unnecessary as all other features can be implemented onto 0.1.

Rollout plan

1. Document amp-nested-menu usage with amp-sidebar within 0.1 (Document amp-nested-menu usage in amp-sidebar's README.  #26175 )
2. Merge implemented amp-sidebar a11y improvements from 0.2 into 0.1
3. Remove 0.2 code and the amp-sidebar-v2 experiment
4. Implement non-breaking features (more a11y improvements and better animations) inside 0.1 whenever possible

/cc @ampproject/wg-approvers

[cathyxz]

Approved

[newmuis]

What exactly is launching under 0.1, then? Is there new functionality (like swipe detection) being added to the mask? Is the mask location itself in the DOM being changed?

[dreamofabear]

Was amp-sidebar-v2 experiment ever launched? If no, do we still need a formal removal/approval?

[cathyxz]

IIRC, the experiment never launched. I advocated directly removing it after we removed the documentation, but not sure what the current process is anymore. I was mostly joking about the "approve". =P

Re: @newmuis, we implemented / intend to implement the following:

• Swipe to dismiss (ported back to 0.1)
• amp-nested-menu as a child of amp-sidebar.
• @wassgha 's port of a11y changes, minimal, I'm not actually sure that we should change the role because that could be a breaking change, but that can be discussed in 🚮 Merges amp-sidebar 0.2 into 0.1 and deprecates 0.2 #26177.

Answers to your other questions:

• Swipe detection is implemented on the amp-sidebar element and not the mask:
  

  amphtml/extensions/amp-sidebar/0.1/amp-sidebar.js

  Line 564 in 8eb76ef

  swipeElement: dev().assertElement(this.element),

• @leafsy considered changing the mask location in 0.2, so that we could put it entirely in <amp-sidebar>. I believe there was a discussion tracked in an issue somewhere, where, since amp-story would have to simultaneously support both versions of sidebar for some time, we'd need to expose a public method to the effect of isVersionOne or isMaskInSidebar boolean to the amp-sidebar API to enable amp-story to distinguish between the two. This work was prototyped but not launched. More browser / device testing would be necessary to launch if deemed a worthwhile effort.

To add to @wassgha 's comment:

• Push animations were prototyped here: [DO NOT MERGE] Prototype push animation for sidebar v0.2 #25358 (demo: https://amp-sidebar-animation.firebaseapp.com/ ), but needed to go through more thorough testing on multiple devices / browsers before launch.

Happy New Year!

[wassgha]

/cc @caroqliu
Thanks @cathyxz !

[wassgha]

If #26958 happens, this should move forward

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.