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

Update sidebar v0.2 + nested menu and remove history API #25048

Merged
merged 15 commits into from
Oct 24, 2019

Conversation

leafsy
Copy link
Contributor

@leafsy leafsy commented Oct 14, 2019

I2I: #25049

resolves #24382, #24668

This PR makes the following updates to support the new version of amp-sidebar:

  • Change version from 1.0 to 0.2;
  • Change name of nested menu component to <amp-nested-menu>;
  • Remove history API from nested menu;
  • Add unit tests for nested menu;
  • Improve a11y for both components by using role="dialog" and shifting focus;

@amp-owners-bot
Copy link

Hey @rcebulko, these files were changed:

  • extensions/amp-nested-menu/OWNERS

Copy link
Contributor

@rcebulko rcebulko left a comment

Choose a reason for hiding this comment

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

Approving new OWNERS file

Copy link
Contributor

@cathyxz cathyxz left a comment

Choose a reason for hiding this comment

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

LGTM

'amp-drilldown-submenu',
'amp-drilldown-submenu-open',
'amp-drilldown-submenu-close',
// Attributes for amp-nested-menu.
Copy link
Contributor

@cathyxz cathyxz Oct 23, 2019

Choose a reason for hiding this comment

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

@choumx could you take a look at this for owners bot?
This is a strict renaming of an unlaunched component.

Choose a reason for hiding this comment

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

LGTM

@leafsy leafsy merged commit d524fbc into ampproject:master Oct 24, 2019
@leafsy leafsy deleted the sidebar-v2 branch October 24, 2019 22:30
@leafsy leafsy restored the sidebar-v2 branch October 31, 2019 21:34
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
…25048)

* change sidebar version, update naming and remove history api from nested menu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate using <dialog> aria-modal for sidebar 1.0
5 participants