Add Khanmigo theming for buttons with icons #2088
Conversation
🦋 Changeset detectedLatest commit: d73cff9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
|
Size Change: -753 B (-1%) Total Size: 92.2 kB
ℹ️ View Unchanged
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2088 +/- ##
==========================================
+ Coverage 97.01% 97.08% +0.06%
==========================================
Files 241 243 +2
Lines 27540 28055 +515
Branches 2415 2366 -49
==========================================
+ Hits 26719 27236 +517
+ Misses 821 819 -2
... and 18 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-hzzwrllmcr.chromatic.com/ Chromatic results:
|
nishasy
changed the title
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (116b3f7) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2088"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2088 |
| @@ -271,10 +287,41 @@ const themedSharedStyles: ThemedStylesFn<ButtonThemeContract> = (theme) => ({ | |||
| }, | |||
| startIcon: { | |||
| marginInlineEnd: theme.padding.small, | |||
| marginInlineStart: -theme.padding.small, | |||
There was a problem hiding this comment.
thought: One thing that got me thinking is that marginInlineStart (and end) is not supported by Safari v14 (we currently have to support this version). I think it would be fine to keep it this way as we haven't gotten any reports about this, but it's worth keeping an eye on it.
Hopefully we'll update our support list to support Safari v15+ in the near future.
| </Button> | ||
| <Strut size={Spacing.medium_16} /> | ||
| <Button | ||
| size="small" |
There was a problem hiding this comment.
Adding small buttons to story
…or browser support, minor cleanup
Summary:
We want to allow buttons to have a khanmigo-themed version of the icons.
The idea is that icons will be wrapped by a rounded container and have
different focus/hover states (probably worth defining a pressed state as well).
Still waiting on design confirmation that this looks right when there
are icons on both sides.
More info in this slack thread:
https://khanacademy.slack.com/archives/C8Z9DSKC7/p1697490308355979
Issue: https://khanacademy.atlassian.net/browse/WB-1631
Test plan:
Manual testing on Storybook (/button--khanmigo-theme)
For both the regular theme and the Khanmigo theme:
Khanmigo theme:
tertiary buttons.