WB-1847: Dropdown - Update SelectOpener to match design specs #2422
Conversation
🦋 Changeset detectedLatest commit: 3ae0350 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 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: +131 B (+0.14%) Total Size: 96.6 kB
ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-ujhwtppdod.chromatic.com/ Chromatic results:
|
| ? disabled || error | ||
| ? "currentColor" | ||
| : tokens.color.white |
There was a problem hiding this comment.
note: this was no longer needed, so I got rid of this (now light uses a similar style compared to the default version).
| // These values are default padding (16 and 12) minus 1, because | ||
| // changing the borderWidth to 2 messes up the button width | ||
| // and causes it to move a couple pixels. This fixes that. | ||
| const adjustedPaddingLeft = tokens.spacing.medium_16 - 1; | ||
| const adjustedPaddingRight = tokens.spacing.small_12 - 1; |
There was a problem hiding this comment.
note: This hack is no longer needed now that we use outline instead of border.
| cursor: "not-allowed", | ||
| ":focus-visible": { | ||
| boxShadow: `0 0 0 1px ${tokens.color.white}, 0 0 0 3px ${tokens.color.offBlack32}`, |
There was a problem hiding this comment.
note: Also changed this box-shadow to outline for consistency.
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 (9bc83b5) 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="PR2422"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2422 |
| @@ -9,10 +9,12 @@ export const semanticColor = { | |||
| primary: { | |||
| default: color.blue, | |||
| active: color.activeBlue, | |||
| pressing: color.fadedBlue, | |||
There was a problem hiding this comment.
question (non-blocking): In our tokens meeting last week, we were chatting about active vs pressed naming and decided to go with the pressed (or is it press) naming since it doesn't have to match the CSS names. I see we have active and pressing token names from the original semantic colors in Figma. Do we want to start moving towards that, or is that a later thing we want to handle?
There was a problem hiding this comment.
Thanks for bringing this up! As we discussed in the WB cubby meeting, I'm going to rename this to press.foreground and active will change to press.background.
| // Provides an outline around the button to match the border width | ||
| outlineOffset: -(tokens.border.width.thin + borderWidth), | ||
| outlineStyle: "solid", | ||
| outlineWidth: tokens.border.width.thin, |
There was a problem hiding this comment.
The outline in Figma visually is 3px wide instead of 2! In Figma, it's shown with a 2px border and a shadow with 1px spread (I think it's fine we've simplified it to using only an outline!). We don't have a border width token for 3px though. Would this be a one-off, a new token, or an update to the designs so it uses 2px instead? Let me know what you think!
The error outline when light=true is 2px though!
There was a problem hiding this comment.
hmmm yeah, tbh that's a bit confusing. In Figma, they are using border: inset, 2px for the normal outline, but I see it translated as 3px. I'll bring this topic today to our WB sync meeting.
There was a problem hiding this comment.
Also, based on the last WB cubby discussion, I'll use the same focus that we use with TextField/TextArea for better consistency.
Summary:
The dropdown opener currently uses a darkBlue/transparent background and that
could cause accessibility issues (color contrast). These styles are not matching
the current Figma specs, which use a white background.
Also, took the opportunity to switch from
colortosemanticColorin most ofthe cases of that component.
Specific style changes:
darkBlueto awhitebackground, and also changed thedisabledstate to use disabled semantic colors instead of blue shades.bordertooutlinewhen focused/active for better consistency across the library.activeBluetext color.New
semanticColortokensAdditionally, I've added a couple of new tokens in
semanticColorto includesome missing colors that are defined in Figma.
https://www.figma.com/design/VbVu3h2BpBhH80niq101MHHE/%F0%9F%92%A0-Main-Components?node-id=17450-482&t=jCX4iO74AL9fsTwh-4
Issue: https://khanacademy.atlassian.net/browse/WB-1847
Test plan:
Navigate to
/?path=/story/packages-dropdown-singleselect--lightand verify thatthe SelectOpener component now has a white background.
Also verify all the variants in
/?path=/docs/packages-dropdown-singleselect-all-variants--docs