-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore(ui): migrate Select component to TypeScript #571
Conversation
🦋 Changeset detectedLatest commit: fbbad56 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting to review this component now. Here's an initial observation regarding imports. Not a blocking change at all, but would love to see the imports cleaned up and default directory exports utilised.
packages/ui-components/src/components/SelectOption/SelectOption.component.tsx
Show resolved
Hide resolved
@guoda-puidokaite regarding export/import lets focus on migration make same way it iwas before then we would decide where we wanna go with it later with whoel project. |
Yah, not a blocking change for this PR as I mentioned. 🤪 But wanted to bring it up and see what you think. We can talk about it in the future. 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks greattt! 🚀
Just had a few curious questions, code improvements and one type addition (most important requested change for ticket).
Thank You!
packages/ui-components/src/components/Select/Select.component.tsx
Outdated
Show resolved
Hide resolved
packages/ui-components/src/components/SelectDivider/SelectDivider.component.tsx
Outdated
Show resolved
Hide resolved
packages/ui-components/src/components/SelectOption/SelectOption.component.tsx
Show resolved
Hide resolved
packages/ui-components/src/components/SelectOption/SelectOption.component.tsx
Outdated
Show resolved
Hide resolved
packages/ui-components/src/components/SelectOption/SelectOption.component.tsx
Outdated
Show resolved
Hide resolved
I really think that some change requests are good improvements, but others I would not mix up with migration. Migration is migration, we need to move trhough as carefull and quick as possible. Then later on we could be more dedicated on refactoring topics, but it is nice that you point some staff out @guoda-puidokaite |
As mentioned, only the type addition |
packages/ui-components/src/components/SelectOption/SelectOption.test.tsx
Show resolved
Hide resolved
packages/ui-components/src/components/SelectOption/SelectOption.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super niiice! One more down. Thank Youuu! 💪
packages/ui-components/src/components/FormHint/FormHint.component.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 🚀
Only one small change request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank You! 🚀💪✅
* chore(ui): migrate select component * chore(ui): move FormRow to deprecated * chore(ui): add components to deprecated * chore(ui): last clean up * chore(ui): bring the status quo for FormRow * chore(ui): deprecated change * chore(ui): fix select story * chore(ui): rollback some changes and fix tests * chore(ui): add changeset * chore(ui): update index.js * chore(ui): minor review changes * chore(ui): minor ui issue * chore(ui): remove redundancy * fix(ui): removes html id prop from interface * fix(ui): extend interface to mirror underlying HTML elem props --------- fixes #249 Co-authored-by: Esther Schmitz <[email protected]>
Summary
Select
,SelectOption
,SelectDivider
components toTypeScript
Select
,SelectOption
,Changes Made
TypeScript Migration:
Select
,SelectOption
,SelectDivider
component files toTypeScript
.Select
,SelectOption
component forJavascript
files.SelectRow
componentRefactoring:
Testing:
AppShellProvider
component from tests and stories, as not yet converted. But replace it with PortalProvider cause it should be a more accurate use case, since stories use PortalProvider instead.Storybook
.vitest
.Release:
changeset
file.Related Issues
Testing Instructions
Storybook
Select
,SelectOption
,SelectDivider
testsChecklist