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

Create separator component #113

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Create separator component #113

merged 2 commits into from
Oct 26, 2023

Conversation

germain-gg
Copy link
Contributor

@germain-gg germain-gg commented Oct 23, 2023

Initially, we'll use it just for explicit/standalone <hr> elements.

There are no immediate plans to replace generic borders in components with standalone, nested divider elements.

@germain-gg germain-gg requested a review from a team as a code owner October 23, 2023 14:11
@germain-gg germain-gg requested review from dbkr and weeman1337 and removed request for a team October 23, 2023 14:11

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Oct 23, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 78b5ff7
Status: ✅  Deploy successful!
Preview URL: https://6513f04d.compound-web.pages.dev
Branch Preview URL: https://germain-gg-divider.compound-web.pages.dev

View logs

@janogarcia
Copy link

janogarcia commented Oct 23, 2023

Initially, we'll use it just for explicit/standalone <hr> elements.

Unsure about the benefits of replacing most border decorations (for example, in menus) with a nested instance of a divider component/element, as long as we can apply semantic tokens for those border decorations.

@janogarcia
Copy link

I'll edit the description to reflect that.

@janogarcia janogarcia changed the title Create separator component Create divider component Oct 23, 2023
@germain-gg germain-gg changed the title Create divider component Create separator component Oct 23, 2023
@germain-gg germain-gg requested a review from t3chguy October 23, 2023 18:03
@germain-gg germain-gg enabled auto-merge (squash) October 23, 2023 18:04
Comment on lines +57 to +59
// Casting to `RefObject` as everything here should use this type.
// Functions refs are not something we wish to support and string refs
// are already unsupported with `forwardRef`.
Copy link
Member

Choose a reason for hiding this comment

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

If you're interested, an alternative solution that makes it possible to support function refs would be to create a new const ownRef = useRef(), and then merge it with the forwarded ref

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm really cool! Didn't know that existed, thank you!

@germain-gg germain-gg merged commit eb42599 into main Oct 26, 2023
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.

None yet

4 participants