-
Notifications
You must be signed in to change notification settings - Fork 17
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
Tweaks to v2 components based on Sam's notes #1649
Conversation
|
Visit the preview URL for this PR (updated for commit 4463753): https://penumbra-ui-preview--pr1649-jessepinho-component-uhxpia21.web.app (expires Tue, 13 Aug 2024 05:25:15 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 709d729610ef7a6369b23f1cb2b820a60cc685b1 |
@@ -88,24 +96,31 @@ export const ButtonGroup = ({ | |||
*/} | |||
{isIconOnly(props) && | |||
props.buttons.map((button, index) => ( | |||
<ButtonPriorityContext.Provider key={index} value={index === 0 ? 'primary' : 'secondary'}> |
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.
We got rid of the context, so now we just pass the priority directly to the button.
<Density compact> | ||
<RadixDialog.Close asChild> | ||
<Button icon={X} iconOnly priority='secondary'> | ||
Close | ||
</Button> | ||
</RadixDialog.Close> | ||
</Density> |
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.
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.
Dialog is 🔥🔥🔥
Based on a recent convo with Sam, we decided to make some a few tweaks to existing v2 components:
primary
priority by default. There are a number of contexts in which they can besecondary
. Thus,priority
is now a prop, rather than a context, to make it easier to set priority.primary
, because there are many cases where all buttons in a group will be of equal priority (e.g., the Staking page, where someone can delegate or undelegate).hasPrimaryButton
boolean prop. When set totrue
, the first button in a button group will be marked asprimary
.iconOnly='adornment'
, there will no longer be a focus outline on the button when it is clicked, since it didn't really look good.<Button />
for closing a dialog.