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

[MulitSelect, SingleSelect] Move showOpenerLabelAsText out of sharedProps #2379

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

daniellewhyte
Copy link
Contributor

@daniellewhyte daniellewhyte commented Dec 4, 2024

Summary:

Follow up from #2354 the new showOpenerLabelAsText was included in the sharedProps in the MultiSelect and SingleSelect that are passed to the SelectOpener which caused many unit tests to fail when trying to update the dropdown package in webapp (https://github.com/Khan/webapp/pull/27791)

Issue: XXX-XXXX

Test plan:

Install the snapshot in webapp, yarn test

@daniellewhyte daniellewhyte self-assigned this Dec 4, 2024
Copy link

changeset-bot bot commented Dec 4, 2024

🦋 Changeset detected

Latest commit: 0371268

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

Copy link
Contributor

github-actions bot commented Dec 4, 2024

Size Change: 0 B

Total Size: 101 kB

ℹ️ View Unchanged
Filename Size
packages/wonder-blocks-accordion/dist/es/index.js 3.78 kB
packages/wonder-blocks-banner/dist/es/index.js 1.53 kB
packages/wonder-blocks-birthday-picker/dist/es/index.js 1.77 kB
packages/wonder-blocks-breadcrumbs/dist/es/index.js 887 B
packages/wonder-blocks-button/dist/es/index.js 4.04 kB
packages/wonder-blocks-cell/dist/es/index.js 2.01 kB
packages/wonder-blocks-clickable/dist/es/index.js 3.06 kB
packages/wonder-blocks-core/dist/es/index.js 3.44 kB
packages/wonder-blocks-data/dist/es/index.js 6.24 kB
packages/wonder-blocks-dropdown/dist/es/index.js 18.4 kB
packages/wonder-blocks-form/dist/es/index.js 6.28 kB
packages/wonder-blocks-grid/dist/es/index.js 1.36 kB
packages/wonder-blocks-i18n/dist/es/index.js 4.77 kB
packages/wonder-blocks-icon-button/dist/es/index.js 3 kB
packages/wonder-blocks-icon/dist/es/index.js 871 B
packages/wonder-blocks-labeled-field/dist/es/index.js 72 B
packages/wonder-blocks-layout/dist/es/index.js 1.82 kB
packages/wonder-blocks-link/dist/es/index.js 2.28 kB
packages/wonder-blocks-modal/dist/es/index.js 5.37 kB
packages/wonder-blocks-pill/dist/es/index.js 1.65 kB
packages/wonder-blocks-popover/dist/es/index.js 4.87 kB
packages/wonder-blocks-progress-spinner/dist/es/index.js 1.52 kB
packages/wonder-blocks-search-field/dist/es/index.js 1.3 kB
packages/wonder-blocks-switch/dist/es/index.js 1.94 kB
packages/wonder-blocks-testing-core/dist/es/index.js 3.74 kB
packages/wonder-blocks-testing/dist/es/index.js 1.07 kB
packages/wonder-blocks-theming/dist/es/index.js 693 B
packages/wonder-blocks-timing/dist/es/index.js 1.8 kB
packages/wonder-blocks-tokens/dist/es/index.js 2.36 kB
packages/wonder-blocks-toolbar/dist/es/index.js 905 B
packages/wonder-blocks-tooltip/dist/es/index.js 7.08 kB
packages/wonder-blocks-typography/dist/es/index.js 1.23 kB

compressed-size-action

Copy link
Contributor

github-actions bot commented Dec 4, 2024

A new build was pushed to Chromatic! 🚀

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

Chromatic results:

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

@daniellewhyte daniellewhyte marked this pull request as ready for review December 4, 2024 18:25
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/wonder-blocks for changes to .changeset/famous-buckets-vanish.md, packages/wonder-blocks-dropdown/src/components/multi-select.tsx, packages/wonder-blocks-dropdown/src/components/single-select.tsx

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

@khan-actions-bot khan-actions-bot requested a review from a team December 4, 2024 18:25
@daniellewhyte daniellewhyte removed the request for review from a team December 4, 2024 18:25
Copy link
Contributor

github-actions bot commented Dec 4, 2024

npm Snapshot: Published

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

Packages can also be installed manually by running:

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

@daniellewhyte
Copy link
Contributor Author

Not ready for review yet, so I removed WB team. This is open so I can get the npm snapshot. Still need to test against webapp.

@beaesguerra
Copy link
Member

Good catch @daniellewhyte ! I'm also working on some SingleSelect and MultiSelect changes! One of the PRs involves changing these to functional components and it will address this issue (showOpenerLabelAsText is moved out of the sharedProps for the opener): SingleSelect change, MultiSelect change

I'm going through some final testing/reviews for other changes but it'll include this fix when that's wrapped up (in case you wanted to leave things as is until then!)

@daniellewhyte
Copy link
Contributor Author

@beaesguerra Do you have an ETA on when that's expected to hit webapp? Perseus syncs its dependencies from webapp, so I was trying to deploy the updated the dropdown package sometime this week.

@khan-actions-bot khan-actions-bot requested a review from a team December 4, 2024 20:13
@beaesguerra
Copy link
Member

@daniellewhyte Sounds good! We can get this PR through sooner then 😄 (some of the upcoming work is still being reviewed!)

Copy link
Member

@beaesguerra beaesguerra left a comment

Choose a reason for hiding this comment

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

Looks good, just noticed a small thing in the changeset file! Thanks for fixing this!

"@khanacademy/wonder-blocks-dropdown": patch
---

[MultiSelect] Remove `showOpenerLabelAsText` from sharedProps that are passed to SelectOpener
Copy link
Member

Choose a reason for hiding this comment

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

minor: it will also be updated in SingleSelect!

Suggested change
[MultiSelect] Remove `showOpenerLabelAsText` from sharedProps that are passed to SelectOpener
[MultiSelect and SingleSelect] Remove `showOpenerLabelAsText` from sharedProps that are passed to SelectOpener

@daniellewhyte daniellewhyte merged commit c8b5b2e into main Dec 4, 2024
14 checks passed
@daniellewhyte daniellewhyte deleted the fix-new-dropdown-prop branch December 4, 2024 21:12
Copy link

codecov bot commented Dec 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (2b8424c) to head (0371268).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@     Coverage Diff      @@
##   main   #2379   +/-   ##
============================
============================

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 2b8424c...0371268. Read the comment docs.

@daniellewhyte daniellewhyte changed the title [MulitSelect] Move showOpenerLabelAsText out of sharedProps [MulitSelect, SingleSelect] Move showOpenerLabelAsText out of sharedProps Dec 4, 2024
beaesguerra pushed a commit that referenced this pull request Dec 16, 2024
## Summary:
Follow up from #2354 the new `showOpenerLabel` was included in the `sharedProps`  in the MultiSelect that are passed to the SelectOpener which caused many unit tests to fail when trying to update the dropdown package in webapp (Khan/webapp#27791)

Issue: XXX-XXXX

## Test plan:
Install the snapshot in webapp, yarn test

Author: daniellewhyte

Reviewers: beaesguerra

Required Reviewers:

Approved By: beaesguerra

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️  dependabot

Pull Request URL: #2379
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants