-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add new pricing options component #530
Add new pricing options component #530
Conversation
🦋 Changeset detectedLatest commit: 919fd01 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🟢 No design token changes found |
|
5edd985
to
9f6330c
Compare
9f6330c
to
d129011
Compare
* Once subgrid is supported in the last 3 major versions of Safari (i.e., when Safari 18 is released), it can be fully removed. | ||
* https://caniuse.com/css-subgrid | ||
*/ | ||
@supports not (grid-template-rows: subgrid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be done as a progressive enhancement, where using subgrid is optional and opt-in? Looks like it's the other way around right now, which might not fly with house rules. That said, this is much nicer for balancing our tech debt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented the progressive enhancement approach initially but chose this route because:
- It's as safe as the progressive enhancement: the
@supports
at-rule is supported on all our supported browsers 🤪, which ensures everyone is getting the "baseline" behavior - It is much simpler and easier to read than the progressive enhancement implementation. And we can delete the whole statement when
subgrid
is fully supported.
Co-authored-by: Rez <[email protected]>
Co-authored-by: Rez <[email protected]>
Co-authored-by: Rez <[email protected]>
…-pricing-card-component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work @danielguillan. It's a big PR, so I've focussed on providing critical path feedback only so we can get this out today. Anything extra can be updated in follow up PR's or uncovered during smoke tests.
Summary
Part of https://github.com/github/primer/issues/2954
Adds new pricing options component.
🔗 Documentation
🔗 Storybook
📋 Todo
Default
storyList of notable changes:
What should reviewers focus on?
Steps to test:
Contributor checklist:
Reviewer checklist:
Screenshots: