Skip to content

✨ [bento][amp-sidebar] Initial AMP Component#31593

Merged
krdwan merged 6 commits intoampproject:masterfrom
krdwan:amp-sidebar-amp
Dec 16, 2020
Merged

✨ [bento][amp-sidebar] Initial AMP Component#31593
krdwan merged 6 commits intoampproject:masterfrom
krdwan:amp-sidebar-amp

Conversation

@krdwan
Copy link
Copy Markdown
Contributor

@krdwan krdwan commented Dec 14, 2020

Partial: #31366

Initial AMP Component of amp-sidebar

Todo: tests

}

/** @override */
mutationObserverCallback() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Allow opening / closing of sidebar by adding / removing the open attribute on amp-sidebar element.

</Sidebar>
<div style={{marginTop: 8}}>
<br />
<button onClick={() => ref.current.toggle()}>toggle</button>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed this to match style of other Storybooks (i.e. Lightbox)

Copy link
Copy Markdown
Contributor

@dvoytenko dvoytenko left a comment

Choose a reason for hiding this comment

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

Looks good. We can proceed in this scope or immediately catch up this to the work done in the #31565 for style customization.

@krdwan krdwan marked this pull request as ready for review December 15, 2020 22:31
Copy link
Copy Markdown
Contributor

@dvoytenko dvoytenko left a comment

Choose a reason for hiding this comment

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

LGTM with a nit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants