Skip to content

Conversation

@Westbrook
Copy link
Contributor

Description

Outline style requirements for content that is thrown into an overlay.

PTAL: https://westbrook-overlay-styles--spectrum-web-components.netlify.app/components/overlay-trigger

Related Issue

fixes #1267

Motivation and Context

Make adopting out elements as easy as possible.

Screenshots (if appropriate):

image

Types of changes

  • Docs

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

}
customElements.define('docs-component', ComponentElement);

class StyledElement extends HTMLElement {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code sample in the Docs are copyable, but not runnable inline, so the definition needs to actually be set here.

aashritandon92
aashritandon92 previously approved these changes Apr 16, 2021
Copy link
Contributor

@aashritandon92 aashritandon92 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 to me!


```html
<overlay-trigger placement="top-start">
<button slot="trigger">Trigger Element</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, was the usage of HTML button intentional, since rest of the document uses sp-button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! I was thinking this helped reduce the perception of "magic" here so that readers could focus on the [slot="click-content"] element, but maybe you bringing this up shows that I was mistaken and it should be sp-button for uniformity?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, sp-button would be better in my opinion.

Copy link
Contributor

@aashritandon92 aashritandon92 left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@Westbrook Westbrook merged commit 2e23d5a into main Apr 20, 2021
@Westbrook Westbrook deleted the westbrook/overlay-styles branch April 20, 2021 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Overlay] Clarify style/theme resolution/throwing in the documentation

3 participants