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.

Sign up for GitHub

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

Jump to bottom

refactor(typescript): convert Dropdown and dependencies to TypeScript #13022

Merged
merged 8 commits into from
Jan 30, 2023
Merged

refactor(typescript): convert Dropdown and dependencies to TypeScript #13022

merged 8 commits into from
Jan 30, 2023

Conversation

lewandom
Copy link
Contributor

@lewandom lewandom commented Jan 24, 2023 •
edited by tay1orjones
Loading

Closes #12542
Closes #13048

Convert React Dropdown component and the dependencies (FluidForm and ListBox) to add TypeScript support.

Changelog

Changed

  • Converted relevant component files from *.js to .ts/.tsx.
  • Introduced appropriate TypeScript types for components (component types, props types) and variables (e.g. event handlers arguments).
  • Addressed a small issue in Dropdown click handler related to use of focus() function.

Testing / Reviewing

The Dropdown component imports for the storybook should be valid and rendering. In a TypeScript environment, you should be able to import and reference the Dropdown component and its typings (DropdownProps, OnChangeData).

@lewandom lewandom requested a review from a team as a code owner January 24, 2023 22:06
@github-actions
Copy link
Contributor

github-actions bot commented Jan 24, 2023 •
edited
Loading

DCO Assistant Lite bot All contributors have signed the DCO.

@lewandom lewandom force-pushed the typescript-dropdown branch from 1ed409a to 868f5af Compare January 24, 2023 22:10
@netlify
Copy link

netlify bot commented Jan 24, 2023

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1ed409a
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/63d0566b3399890008a2e650
😎 Deploy Preview https://deploy-preview-13022--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@lewandom
Copy link
Contributor Author

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

@netlify
Copy link

netlify bot commented Jan 24, 2023

✅ Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 1ed409a
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/63d0566b42e6e200088bc20e
😎 Deploy Preview https://deploy-preview-13022--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

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
Copy link

netlify bot commented Jan 24, 2023

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 868f5af
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/63d0575ad9282a00092803d0
😎 Deploy Preview https://deploy-preview-13022--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

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
Copy link

netlify bot commented Jan 24, 2023

✅ Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 868f5af
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/63d0575a76367d000b8f712e
😎 Deploy Preview https://deploy-preview-13022--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

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
Copy link

netlify bot commented Jan 24, 2023 •
edited
Loading

✅ Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit 70d9708
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/63d847fa04e1ee00089e05cb
😎 Deploy Preview https://deploy-preview-13022--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

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
Copy link

netlify bot commented Jan 24, 2023 •
edited
Loading

✅ Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 70d9708
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/63d847fa6717280009e29175
😎 Deploy Preview https://deploy-preview-13022--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

francinelucca
francinelucca requested changes Jan 25, 2023
View reviewed changes
Copy link
Collaborator

@francinelucca francinelucca left a comment

Choose a reason for hiding this comment

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

Thanks for getting this PR up! This is looking good, just some change requests and a couple questions.

packages/react/src/components/ListBox/ListBoxSelection.tsx Outdated Show resolved Hide resolved
packages/react/src/components/ListBox/ListBoxSelection.tsx Show resolved Hide resolved
@lewandom lewandom force-pushed the typescript-dropdown branch from 260171d to 451fea4 Compare January 26, 2023 06:24
@lewandom lewandom requested review from francinelucca and removed request for tay1orjones January 26, 2023 16:49
@lewandom lewandom force-pushed the typescript-dropdown branch from 6535663 to b5eabae Compare January 26, 2023 16:58
@lewandom
Copy link
Contributor Author

@francinelucca I fixed the formatting failure. Looks like I also accidentally removed @tay1orjones from the reviewers list...

@jpsorensen
Copy link
Contributor

Looks like I also accidentally removed @tay1orjones from the reviewers list...

Did that happen when you re-requested review? I had a similar thing happen and I'm almost sure I didn't remove the second reviewer myself.

@lewandom
Copy link
Contributor Author

Looks like I also accidentally removed @tay1orjones from the reviewers list...

Did that happen when you re-requested review? I had a similar thing happen and I'm almost sure I didn't remove the second reviewer myself.

@jpsorensen yes, that's what I did and that's what's happened.

tay1orjones
tay1orjones approved these changes Jan 30, 2023
View reviewed changes
Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

Looks great, thank you! 🏆

@francinelucca francinelucca mentioned this pull request Jan 30, 2023
Open
1 task
@kodiakhq kodiakhq bot merged commit 51eae8f into carbon-design-system:main Jan 30, 2023
@lewandom lewandom deleted the typescript-dropdown branch January 31, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francinelucca francinelucca francinelucca approved these changes

@tay1orjones tay1orjones tay1orjones approved these changes

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add TypeScript types to FluidForm Add TypeScript types to Dropdown
6 participants
@lewandom @jpsorensen @tay1orjones @francinelucca @andreancardona @aledavila