Skip to content

📖 bento-base-carousel documentation#35645

Merged
caroqliu merged 5 commits intoampproject:mainfrom
caroqliu:bento-doc
Aug 18, 2021
Merged

📖 bento-base-carousel documentation#35645
caroqliu merged 5 commits intoampproject:mainfrom
caroqliu:bento-doc

Conversation

@caroqliu
Copy link
Copy Markdown
Contributor

@caroqliu caroqliu commented Aug 12, 2021

Context: extensions/amp-base-carousel/amp-base-carousel.md currently contains both AMP-specific and Bento documentation under "Standalone usage" for the 1.0 version of amp-base-carousel.

This PR creates a new README.md (alternative naming suggestions welcome) file inside extensions/amp-base-carousel/1.0 with two parts:

  • Web Component which takes the "Standalone usage" portions from the original amp-base-carousel.md
  • Preact/React Component which duplicates much of the existing documentation with minor updates with regards to casing (attributes -> props) and API differences.

The purpose of this PR is to iron out formatting details across the three modes of usage, including the best way to represent documentation across usage modes. Another purpose is to provide a README.md file that is completely relevant to and publishable on npm.

Note: This PR does not actually rename the Bento AMP implementation of amp-bento-carousel to bento-base-carousel or BaseCarousel to BentoBaseCarousel. The former is handled in #34820 and the latter should be handled separately.

Comment on lines +179 to +195
Each Bento component has a small CSS library you must include to guarantee proper loading without [content shifts](https://web.dev/cls/). Because of order-based specificity, you must manually ensure that stylesheets are included before any custom styles.

```html
<link rel="stylesheet" type="text/css" href="https://cdn.ampproject.org/v0/amp-base-carousel-1.0.css">
```

Alternatively, you may also make the light-weight pre-upgrade styles available inline:

```html
<style data-bento-boilerplate>
bento-base-carousel {
display: block;
overflow: hidden;
position: relative;
}
</style>
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need to mention this information in the first paragraph as well, since it must be included. Maybe:

You must include each Bento component's required CSS library to guarantee proper loading and before adding custom styles. Or use the light-weight pre-upgrade styles available inline. See Layout and style.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wait, is this untrue for P/React?

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.

For Preact, as long as they give explicit dimensions i.e. width and height on the component, they don't need this.

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.

Is the You must include each Bento component's required CSS library to guarantee proper loading and before adding custom styles. Or use the light-weight pre-upgrade styles available inline. See Layout and style. recommendation for the line to go under the ### Web component header?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification - moving it under the ### Web component header is perfect

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.

4 participants