Skip to content
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

[🔥AUDIT🔥] WB-1645.fix Dropdown: Fix regression in SingleSelect #2146

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

jandrade
Copy link
Member

@jandrade jandrade commented 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

Screenshot 2023-12-15 at 1 52 16 PM

@jandrade jandrade self-assigned this Dec 15, 2023
Copy link

changeset-bot bot commented Dec 15, 2023

🦋 Changeset detected

Latest commit: 53c4f69

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@khanacademy/wonder-blocks-dropdown Patch
@khanacademy/wonder-blocks-birthday-picker Patch

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

@jandrade jandrade added the audit PR is using the audit OLC flow label Dec 15, 2023
@khan-actions-bot khan-actions-bot requested a review from a team December 15, 2023 18:50
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/wonder-blocks for changes to .changeset/warm-hats-walk.md, packages/wonder-blocks-dropdown/src/components/single-select.tsx, packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

@jandrade jandrade requested review from jeremyspangler, nedredmond and jeremywiebe and removed request for jeremyspangler December 15, 2023 18:51
Copy link

codecov bot commented Dec 15, 2023

Codecov Report

Merging #2146 (53c4f69) into main (1b21747) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2146   +/-   ##
=======================================
  Coverage   97.06%   97.06%           
=======================================
  Files         244      244           
  Lines       28256    28256           
  Branches     2461     2421   -40     
=======================================
  Hits        27428    27428           
  Misses        828      828           
Files Coverage Δ
...r-blocks-dropdown/src/components/single-select.tsx 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b21747...53c4f69. Read the comment docs.

Copy link
Contributor

github-actions bot commented Dec 15, 2023

Size Change: -6 B (0%)

Total Size: 91.6 kB

Filename Size Change
packages/wonder-blocks-dropdown/dist/es/index.js 12.3 kB -6 B (0%)
ℹ️ View Unchanged
Filename Size
packages/wonder-blocks-accordion/dist/es/index.js 3.69 kB
packages/wonder-blocks-banner/dist/es/index.js 1.53 kB
packages/wonder-blocks-birthday-picker/dist/es/index.js 1.72 kB
packages/wonder-blocks-breadcrumbs/dist/es/index.js 1.13 kB
packages/wonder-blocks-button/dist/es/index.js 4.27 kB
packages/wonder-blocks-cell/dist/es/index.js 2.24 kB
packages/wonder-blocks-clickable/dist/es/index.js 3.24 kB
packages/wonder-blocks-color/dist/es/index.js 1.15 kB
packages/wonder-blocks-core/dist/es/index.js 3.7 kB
packages/wonder-blocks-data/dist/es/index.js 6.33 kB
packages/wonder-blocks-form/dist/es/index.js 5.34 kB
packages/wonder-blocks-grid/dist/es/index.js 1.36 kB
packages/wonder-blocks-i18n/dist/es/index.js 4.54 kB
packages/wonder-blocks-icon-button/dist/es/index.js 3.21 kB
packages/wonder-blocks-icon/dist/es/index.js 1.06 kB
packages/wonder-blocks-labeled-field/dist/es/index.js 72 B
packages/wonder-blocks-layout/dist/es/index.js 1.88 kB
packages/wonder-blocks-link/dist/es/index.js 2.54 kB
packages/wonder-blocks-modal/dist/es/index.js 5.32 kB
packages/wonder-blocks-pill/dist/es/index.js 1.03 kB
packages/wonder-blocks-popover/dist/es/index.js 4.38 kB
packages/wonder-blocks-progress-spinner/dist/es/index.js 1.51 kB
packages/wonder-blocks-search-field/dist/es/index.js 1.55 kB
packages/wonder-blocks-spacing/dist/es/index.js 158 B
packages/wonder-blocks-switch/dist/es/index.js 2.06 kB
packages/wonder-blocks-testing/dist/es/index.js 3.94 kB
packages/wonder-blocks-theming/dist/es/index.js 1.21 kB
packages/wonder-blocks-timing/dist/es/index.js 1.78 kB
packages/wonder-blocks-toolbar/dist/es/index.js 862 B
packages/wonder-blocks-tooltip/dist/es/index.js 5.05 kB
packages/wonder-blocks-typography/dist/es/index.js 1.49 kB

compressed-size-action

Copy link
Contributor

github-actions bot commented Dec 15, 2023

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (7c94d52) 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="PR2146"

Packages can also be installed manually by running:

yarn add @khanacademy/wonder-blocks-<package-name>@PR2146

Copy link
Contributor

github-actions bot commented Dec 15, 2023

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-tdlfyvvctz.chromatic.com/

Chromatic results:

Metric Total
Captured snapshots 324
Tests with visual changes 1
Total stories 402
Inherited (not captured) snapshots [TurboSnap] 0
Tests on the build 324

Copy link
Member

@jeresig jeresig left a comment

Choose a reason for hiding this comment

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

Ahh - good call!

Copy link
Contributor

@jeremywiebe jeremywiebe left a comment

Choose a reason for hiding this comment

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

Thank-you for the quick fix!

@jandrade jandrade merged commit 5d34b4b into main Dec 15, 2023
24 checks passed
@jandrade jandrade deleted the dropdown-fix-1-item branch December 15, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit PR is using the audit OLC flow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants