Add TypeScript annotations to MultiSelect #13144

awarrier99 · 2023-02-13T23:10:09Z

Convert existing propType definitions to TypeScript annotations on the MultiSelect component. This is part of a broader effort to add TypeScript annotations to components, tracked in #12513.

Changelog

Changed

MultiSelect.js has been converted to MultiSelect.tsx
Added awarrier99 to contributor list
Changed a MultiSelect story to include required prop label
Changed a MultiSelect test to return an empty span instead of an empty string for itemToElement (a string is not valid as a JSX constructor, which is how the prop is used in the component)
Changed MultiSelect sorting options to use locale prop, rather than hardcoded 'en'

Testing / Reviewing

In the packages/react directory, run yarn build, yarn storybook. In the storybook, navigate to MultiSelect and confirm that the component's behavior remains unchanged.
In MultiSelect.stories.js and MultiSelect-test.js, add // @ts-check as the first line in the file (temporarily, for the sake of this test). Confirm that the files do not have compilation errors (related to the MultiSelect component). Make changes to the components in those files - change number to string, etc - and confirm that compilation errors appear.

github-actions · 2023-02-13T23:10:26Z

DCO Assistant Lite bot All contributors have signed the DCO.

awarrier99 · 2023-02-13T23:13:37Z

I have read the DCO document and I hereby sign the DCO.

netlify · 2023-02-13T23:14:29Z

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`2e58fa5`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-components-react/deploys/63eac354a755c200080f1785
😎 Deploy Preview	https://deploy-preview-13144--carbon-components-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2023-02-13T23:15:14Z

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	`2e58fa5`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-elements/deploys/63eac3548a02a10008377700
😎 Deploy Preview	https://deploy-preview-13144--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2023-02-13T23:19:04Z

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`1e8f276`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-components-react/deploys/63f7ef080ff7a800071a908a
😎 Deploy Preview	https://deploy-preview-13144--carbon-components-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2023-02-13T23:20:27Z

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	`f48a21d`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-elements/deploys/63ec115a7da04100084f1f17
😎 Deploy Preview	https://deploy-preview-13144--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

andreancardona · 2023-02-20T21:40:52Z

@awarrier99 Thank you so much for the contribution! Do you mind resolving the merge conflicts please :)

awarrier99 · 2023-02-20T22:23:15Z

@awarrier99 Thank you so much for the contribution! Do you mind resolving the merge conflicts please :)

@andreancardona Sure, they should be resolved now!

packages/react/src/components/MultiSelect/MultiSelect.tsx

tw15egan

LGTM once the all-contributors merge conflict is fixed! 👍 ✅

awarrier99 · 2023-02-22T19:49:51Z

LGTM once the all-contributors merge conflict is fixed! 👍 ✅

Resolved!

awarrier99 · 2023-02-22T19:53:50Z

@andreancardona @tw15egan I noticed with the previous run of the workflows that there was an error with a snapshot saying that the public API shouldn't be changed without a new semver bump - do I need to do something to resolve that?

tw15egan · 2023-02-22T19:58:15Z

@awarrier99 usually means snapshots need to be updated due to the changes made. If the changes are intentional (in this case, yes we are modifying the component footprint quite a bit), you can update snapshots with yarn test -u. None of these changes should be breaking so we are good to go 👍🏻

awarrier99 · 2023-02-22T20:11:00Z

@awarrier99 usually means snapshots need to be updated due to the changes made. If the changes are intentional (in this case, yes we are modifying the component footprint quite a bit), you can update snapshots with yarn test -u. None of these changes should be breaking so we are good to go 👍🏻

Got it, sounds good! I'll leave the snapshots as is then

awarrier99 · 2023-02-22T22:52:43Z

@tw15egan sorry I misunderstood, I thought you were saying we were good to go without needing to update the snaps - thanks for taking care of that

tw15egan · 2023-02-22T22:54:29Z

@awarrier99, no worries, I could have worded it better! I just meant it was okay to update them because we are knowingly making changes to the API 👍🏻 Thanks for contributing 🎉

Convert existing propType definitions to Typescript annotations on the MultiSelect component. This is part of a broader effort to add Typescript annotations to components, tracked in carbon-design-system#12513. Closes carbon-design-system#12547. Only type annotations, no breaking feature changes

tw15egan · 2023-02-23T19:18:50Z

packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap

@@ -4820,127 +4820,10 @@ Map {
        "type": "bool",
      },
      "downshiftProps": Object {
-        "args": Array [


@awarrier99 circling back, is there a way to retain the downshiftProps here? Looking at the Dropdown ts conversion, I do not see any snapshot changes to the downshiftProps. I'm trying to parse the difference in implementation to see what may be causing it 🤔

Reference PR: https://github.com/carbon-design-system/carbon/pull/13022/files

The other snapshot changes are intentional and are fine 👍🏻

Sure, let me take a look at this

@tw15egan this should be resolved now - I had changed the prop type to match what was in the Dropdown component originally, but I've now reverted that change

awarrier99 requested a review from a team as a code owner February 13, 2023 23:10

awarrier99 requested review from tw15egan and andreancardona February 13, 2023 23:10

andreancardona added status: merge conflict 🚫 status: waiting for author's response 💬 status: ready for review 👀 and removed status: merge conflict 🚫 status: waiting for author's response 💬 labels Feb 21, 2023

tw15egan reviewed Feb 22, 2023

View reviewed changes

packages/react/src/components/MultiSelect/MultiSelect.tsx Show resolved Hide resolved

tw15egan mentioned this pull request Feb 22, 2023

fix(ListBox): a11y - move filter tag as sibling of menu trigger, esc key improvements #13173

Merged

tw15egan approved these changes Feb 22, 2023

View reviewed changes

tw15egan force-pushed the add-multiselect-types branch from bd71c83 to d25d9ac Compare February 22, 2023 22:46

tw15egan requested a review from a team as a code owner February 22, 2023 22:46

awarrier99 and others added 3 commits February 23, 2023 11:10

fix(MultiSelect): pass locale prop to sort options

726beef

chore(snapshot): update snapshots

463dac4

tw15egan force-pushed the add-multiselect-types branch from d25d9ac to ccd3a5d Compare February 23, 2023 16:10

Merge branch 'main' into add-multiselect-types

51fb3ee

andreancardona approved these changes Feb 23, 2023

View reviewed changes

andreancardona added status: ready to merge 🎉 and removed status: ready for review 👀 labels Feb 23, 2023

andreancardona enabled auto-merge (squash) February 23, 2023 18:30

tw15egan removed the status: ready to merge 🎉 label Feb 23, 2023

tw15egan disabled auto-merge February 23, 2023 18:30

tw15egan requested changes Feb 23, 2023

View reviewed changes

awarrier99 added 2 commits February 23, 2023 14:53

fix(MultiSelect): add back downshift props

6c8f4bd

Merge branch 'main' into add-multiselect-types

1e8f276

tw15egan approved these changes Feb 23, 2023

View reviewed changes

tw15egan added the status: ready to merge 🎉 label Feb 23, 2023

kodiakhq bot merged commit d791865 into carbon-design-system:main Feb 23, 2023

awarrier99 deleted the add-multiselect-types branch February 23, 2023 23:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TypeScript annotations to MultiSelect #13144

Add TypeScript annotations to MultiSelect #13144

awarrier99 commented Feb 13, 2023 •

edited

Loading

github-actions bot commented Feb 13, 2023 •

edited

Loading

awarrier99 commented Feb 13, 2023

netlify bot commented Feb 13, 2023

netlify bot commented Feb 13, 2023

netlify bot commented Feb 13, 2023 •

edited

Loading

netlify bot commented Feb 13, 2023 •

edited

Loading

andreancardona commented Feb 20, 2023

awarrier99 commented Feb 20, 2023

tw15egan left a comment

awarrier99 commented Feb 22, 2023

awarrier99 commented Feb 22, 2023

tw15egan commented Feb 22, 2023 •

edited

Loading

awarrier99 commented Feb 22, 2023

awarrier99 commented Feb 22, 2023

tw15egan commented Feb 22, 2023

tw15egan Feb 23, 2023

tw15egan Feb 23, 2023

awarrier99 Feb 23, 2023

awarrier99 Feb 23, 2023

Add TypeScript annotations to MultiSelect #13144

Add TypeScript annotations to MultiSelect #13144

Conversation

awarrier99 commented Feb 13, 2023 • edited Loading

Changelog

Testing / Reviewing

github-actions bot commented Feb 13, 2023 • edited Loading

awarrier99 commented Feb 13, 2023

netlify bot commented Feb 13, 2023

✅ Deploy Preview for carbon-components-react ready!

netlify bot commented Feb 13, 2023

✅ Deploy Preview for carbon-elements ready!

netlify bot commented Feb 13, 2023 • edited Loading

✅ Deploy Preview for carbon-components-react ready!

netlify bot commented Feb 13, 2023 • edited Loading

✅ Deploy Preview for carbon-elements ready!

andreancardona commented Feb 20, 2023

awarrier99 commented Feb 20, 2023

tw15egan left a comment

Choose a reason for hiding this comment

awarrier99 commented Feb 22, 2023

awarrier99 commented Feb 22, 2023

tw15egan commented Feb 22, 2023 • edited Loading

awarrier99 commented Feb 22, 2023

awarrier99 commented Feb 22, 2023

tw15egan commented Feb 22, 2023

tw15egan Feb 23, 2023

Choose a reason for hiding this comment

tw15egan Feb 23, 2023

Choose a reason for hiding this comment

awarrier99 Feb 23, 2023

Choose a reason for hiding this comment

awarrier99 Feb 23, 2023

Choose a reason for hiding this comment

awarrier99 commented Feb 13, 2023 •

edited

Loading

github-actions bot commented Feb 13, 2023 •

edited

Loading

netlify bot commented Feb 13, 2023 •

edited

Loading

netlify bot commented Feb 13, 2023 •

edited

Loading

tw15egan commented Feb 22, 2023 •

edited

Loading