Skip to content

add bordered button variant #1900

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

Merged
merged 4 commits into from
Dec 12, 2024
Merged

add bordered button variant #1900

merged 4 commits into from
Dec 12, 2024

Conversation

gumaerc
Copy link
Contributor

@gumaerc gumaerc commented Dec 12, 2024

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/6277

Description (What does it do?)

This PR adds the bordered variant to the Button component and updates the Storybook stories to demonstrate its usage.

Screenshots (if appropriate):

image
image

How can this be tested?

  • Spin up mit-learn on this branch
  • Visit Storybook at http://localhost:6006
  • Verify that the bordered button variant behaves as expected in normal, hover and disabled states according to the designs linked in the issue

@gumaerc gumaerc added the Needs Review An open Pull Request that is ready for review label Dec 12, 2024
@gumaerc gumaerc changed the title add bordered variant add bordered button variant Dec 12, 2024
Comment on lines 189 to 196
":hover:not(:disabled)": {
backgroundColor: colors.lightGray1,
},
":disabled": {
backgroundColor: colors.lightGray2,
border: `1px solid ${colors.silverGrayDark}`,
color: colors.silverGrayDark,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

The color darkens on hover, and the border when disabled should be same color as background. (You're right that Figma says the border color is silverGrayDark, but figma also says the border width is 0. I think setting the width to zero will mess up the height. I don't trust figma css :p)

Suggested change
":hover:not(:disabled)": {
backgroundColor: colors.lightGray1,
},
":disabled": {
backgroundColor: colors.lightGray2,
border: `1px solid ${colors.silverGrayDark}`,
color: colors.silverGrayDark,
},
":hover:not(:disabled)": {
backgroundColor: colors.lightGray1,
color: colors.darkGray2,
},
":disabled": {
backgroundColor: colors.lightGray2,
borderColor: colors.lightGray2
color: colors.silverGrayDark,
},

@gumaerc gumaerc merged commit 6a1b436 into main Dec 12, 2024
11 checks passed
@odlbot odlbot mentioned this pull request Dec 18, 2024
13 tasks
This was referenced Dec 20, 2024
This was referenced Jan 2, 2025
@rhysyngsun rhysyngsun deleted the cg/add-bordered-button-variant branch February 7, 2025 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review An open Pull Request that is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants