WB-1645: Allow custom option item elements #2139
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: f67aa59 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 |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
|
Size Change: +172 B (0%) Total Size: 91.4 kB
ℹ️ View Unchanged
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (3db1f69) 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="PR2139"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2139 |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2139 +/- ##
========================================
Coverage 97.05% 97.06%
========================================
Files 242 242
Lines 28058 28191 +133
Branches 2457 2419 -38
========================================
+ Hits 27232 27363 +131
- Misses 826 828 +2
Continue to review full report in Codecov by Sentry.
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-ctfvfmsmrk.chromatic.com/ Chromatic results:
|
jandrade
commented
Dec 13, 2023
There was a problem hiding this comment.
note: Noticed that we didn't have docs for this, and now that this component is more complex, we should have docs included!
| @@ -172,6 +172,7 @@ const CellCore = (props: CellCoreProps): React.ReactElement => { | |||
| href, | |||
| onClick, | |||
| "aria-label": ariaLabel, | |||
| "aria-selected": ariaSelected, | |||
There was a problem hiding this comment.
note: This is needed by OptionItem so we can know when a given option is selected.
Comment on lines
-36
to
-42
| color={ | ||
| disabled | ||
| ? offBlack32 | ||
| : pressed || hovered || focused | ||
| ? white | ||
| : offBlack | ||
| } |
There was a problem hiding this comment.
note: Removed this as we no longer depend on Clickable/getClickableBehavior and instead we assign this with CSS (defined in OptionItem).
| ? activeBlue | ||
| : blue | ||
| : white; | ||
| const {disabled, selected} = props; | ||
|
|
There was a problem hiding this comment.
note: Similar to check.tsx, these states are now handled directly in OptionItem with css objects.
Comment on lines
210
to
212
| subtitle1={ | ||
| <LabelSmall className="subtitle">{subtitle1}</LabelSmall> | ||
| } |
There was a problem hiding this comment.
note: Adding this custom "LabelSmall.subtitle" wrapper to be able to style the hover state correctly.
jeresig
approved these changes
Dec 14, 2023
jandrade
added a commit
that referenced
this pull request
Dec 15, 2023
🖍 _This is an audit!_ 🖍 ## Summary: Fixes a regression in `SingleSelect` where the opener is not able to use an empty string as a label. In #2139, I changed the logic to use `1 item` for those cases, but turns out that this "empty" value feature is required by Perseus. Issue: WB-1645 ## Test plan: Verify that the "Two with Text" example in the `SingleSelect` storybook page shows empty string as the label for both openers: http://localhost:6061/?path=/story/dropdown-singleselect--two-with-text <img width="1584" alt="Screenshot 2023-12-15 at 1 52 16 PM" src="https://github.com/Khan/wonder-blocks/assets/843075/5a65edeb-334b-43db-9a72-ce2fe2058139"> Author: jandrade Auditors: jeresig, jeremywiebe, nedredmond Required Reviewers: Approved By: jeresig, jeremywiebe Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 16.x), ✅ Lint (ubuntu-latest, 16.x), ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 16.x), ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ⏭ Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ✅ gerald, ✅ codecov/project, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 16.x), ✅ Lint (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 16.x), ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ⏭ Chromatic - Skip on Release PR (changesets), ✅ gerald, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ⏭ dependabot Pull Request URL: #2146
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
OptionItemcomponents by usingDetailCellinternally.ClickableBehaviorfromOptionItemand replaced it withDetailCell(which internally usesClickable).OptionItem.Optionitemchanges:aria-selectedis used to allow theOptionItemto be selectable (via aria attributes).Issue: WB-1645
Test plan:
SingleSelect:
selects them.
Screen.Recording.2023-12-13.at.11.41.07.AM.mov
MultiSelect:
selects them.
Screen.Recording.2023-12-13.at.12.03.08.PM.mov
OptionItem docs:
OptionItemis correct and up to date.