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

fix: support children prop type in row component #347

Merged
merged 1 commit into from
Jul 27, 2023
Merged

fix: support children prop type in row component #347

merged 1 commit into from
Jul 27, 2023

Conversation

jonotrujillo
Copy link
Member

@jonotrujillo jonotrujillo commented Jul 26, 2023 •
edited
Loading

What:

Add missing children prop type definition to Row component.

Fixes #345.

Why:

children prop type was not supported for Row component.
​
How:

Extending component props with PropsWithChildren.
​
Media:

N/A

Checklist:

  • Ready to be merged

martimalek
martimalek approved these changes Jul 27, 2023
View reviewed changes
Copy link
Contributor

@martimalek martimalek left a comment

Choose a reason for hiding this comment

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

LGTM.

Maybe outside the scope of the PR, do we have any opinions on using FC? Should we get rid of them and start typing only props?

@nlopin nlopin merged commit 4620776 into main Jul 27, 2023
5 checks passed
@nlopin nlopin deleted the fix/support-children-prop-type-for-row-component branch July 27, 2023 08:41
github-actions bot pushed a commit that referenced this pull request Jul 27, 2023
@semantic-release-bot
chore(release): 1.32.1 [skip ci]
99e3207 
### [1.32.1](v1.32.0...v1.32.1) (2023-07-27)

### Bug Fixes

* support children prop type in row component ([#347](#347)) ([4620776](4620776))
@github-actions
Copy link
Contributor

🎉 This PR is included in version 1.32.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nlopin
Copy link
Contributor

nlopin commented Jul 27, 2023

LGTM.

Maybe outside the scope of the PR, do we have any opinions on using FC? Should we get rid of them and start typing only props?

We need to choose one approach, currently there are components that do both const Component: FC<Props> = ({...rest}: Props) {...}

@martimalek
Copy link
Contributor

IMO we should only type the props, the component will implicitly be typed as JSX.Element, by getting rid of FC we will also make the migration to React 18 simpler, since FC has implicit children in 17 but not in 18. Check other downsides of FC.

github-actions bot pushed a commit that referenced this pull request Sep 29, 2023
@semantic-release-bot
chore(release): 3.0.0-next.1 [skip ci]
07ca1f9 
## [3.0.0-next.1](v2.1.0-next.2...v3.0.0-next.1) (2023-09-29)

### ⚠ BREAKING CHANGES

* remove weak prop from Text and add codemod for it (#374)

### Features

* **breadcrumbs:** use css variables for colors ([#382](#382)) ([f13658c](f13658c))
* migrate tag and selectlist variant colors to css vars ([#381](#381)) ([e11833c](e11833c))
* remove weak prop from Text and add codemod for it ([#374](#374)) ([df98bb9](df98bb9))
* replace fixed colors with semantic tokens ([204a71d](204a71d))
* tooltip placement codemod ([#377](#377)) ([258d8ed](258d8ed))
* **accordion:** migrate to semantic colors ([37c9df5](37c9df5))
* **banner:** migrate to semantic colors ([5c30334](5c30334))
* **buttons:** use css variables for colors ([1664f57](1664f57))
* **card:** migrate to semantic colors ([bc90913](bc90913))
* **checkbox:** migrate to semantic colors ([18bd927](18bd927))
* **datepicker, infobanner:** migrate to semantic colors ([98f3d53](98f3d53))
* **design-token:** start restructuring the design tokens ([f477ef9](f477ef9))
* **dimming:** migrate to semantic colors ([a47e1a7](a47e1a7))
* **divider:** migrate to semantic colors ([d0bc552](d0bc552))
* **filepicker:** migrate to semantic colors ([5b2e16c](5b2e16c))
* **filePicker:** add alwaysShowActionButton to mobile ([#303](#303)) ([5d46685](5d46685))
* **headline:** migrate to semantic colors ([f23e081](f23e081))
* **icon:** migrate to semantic colors ([e876e7a](e876e7a))
* **InfoBanner:** fix up the InfoBannerCard Link export ([f6dac56](f6dac56))
* **input:** migrate to semantic colors ([3268c02](3268c02))
* **label:** migrate to semantic colors ([fb6b7e1](fb6b7e1))
* **Link:** migrate to semantic colors ([d16e546](d16e546))
* **modal:** migrate to semantic colors ([90e37c5](90e37c5))
* **offcanvas:** migrate to semantic colors ([1a4a9fd](1a4a9fd))
* **pagination:** use css variables for colors ([f85b75b](f85b75b))
* **password:** migrate to semantic colors ([715f23b](715f23b))
* **popover:** migrate to semantic colors ([e8c66f7](e8c66f7))
* **radio:** migrate to semantic colors ([8aa03d7](8aa03d7))
* **search:** migrate to semantic colors ([0908aed](0908aed))
* **select:** migrate to semantic colors ([7ce9d99](7ce9d99))
* **selectlist:** migrate to semantic colors ([ef8426c](ef8426c))
* **skeleton:** migrate to semantic colors ([16211d9](16211d9))
* **spinner:** migrate to semantic colors ([ae68790](ae68790))
* **tabbar:** use css variables for colors ([4512994](4512994))
* **table:** add sortable table components and hook ([362dba5](362dba5))
* **tag:** use css variables for colors ([d71e73b](d71e73b))
* **text:** migrate to semantic colors ([9ad1848](9ad1848))
* **toggle:** migrate to semantic colors ([f81322e](f81322e))
* **tokens:** create color schemas ([6f1e40f](6f1e40f))
* **tooltip:** migrate to semantic colors ([f6d79f6](f6d79f6))
* add grid system components ([#198](#198)) ([c435683](c435683)), closes [#200](#200)
* add InfoBanner component ([#225](#225)) ([e434f5a](e434f5a))
* add missing colors to match Figma file ([#289](#289)) ([b0ae302](b0ae302))
* Add Popover component to wave ([#230](#230)) ([0c49ab3](0c49ab3))
* add secondary variant for text component ([#170](#170)) ([f056fea](f056fea))
* added Accordion component ([#172](#172)) ([439b67c](439b67c))
* adds select page size component ([#201](#201)) ([068b958](068b958))
* implement compound component approach for InfoBanner ([#336](#336)) ([9f1982c](9f1982c))
* introduce `useToggle` hook ([#220](#220)) ([83cbad8](83cbad8))
* popover ([#234](#234)) ([8e4075b](8e4075b))
* remove brand colors from semantic colors object ([#208](#208)) ([1ccb794](1ccb794))
* remove nanoid ([#321](#321)) ([8e952a0](8e952a0))
* Tooltip with react-popper ([#267](#267)) ([6965ef4](6965ef4))
* update missing icons ([#228](#228)) ([2f840e5](2f840e5))

### Bug Fixes

* support children prop type in row component ([#347](#347)) ([4620776](4620776))
* **docs:** remove unused imports ([0c439a5](0c439a5))
* **popover:** add missing semantic color ([0936f95](0936f95))
* **tabbar:** set font-family -> was "Times" in storybook ([4846aa8](4846aa8))
* add support for children props for Label component ([#340](#340)) ([c840c88](c840c88))
* set focus on password input resolve [#124](#124) ([#328](#328)) ([5068920](5068920))
* **datepicker:** invoke `onClose` handler only once ([#300](#300)) ([9190e6c](9190e6c))
* **inline-spinner:** html and a11y  ([#307](#307)) ([cb46551](cb46551)), closes [#306](#306)
* **password:** use `flex-end` instead of `end` ([#291](#291)) ([debdb5a](debdb5a)), closes [#290](#290)
* **tooltip:** type children prop ([#330](#330)) ([9bbdafe](9bbdafe))
* **tooltip:** use correct props with children type ([339fce2](339fce2))
* add layout props to TableHeaderCell and TableCell ([#185](#185)) ([0915c58](0915c58))
* adding layout to the compose styles ([#233](#233)) ([48af1d1](48af1d1))
* cleanup Modal timeout ([#193](#193)) ([fcb1341](fcb1341))
* display semantic colors in table and allow to filter them ([#258](#258)) ([2e83e76](2e83e76))
* export popover component in components index ([#246](#246)) ([0def6b8](0def6b8))
* expose padding prop and adjust z-index for Popover component ([#263](#263)) ([da0e80b](da0e80b))
* giving singlevalue full max width to override the selectlist styles ([#248](#248)) ([115ae70](115ae70))
* replace weak prop with secondary ([#206](#206)) ([37a7f4f](37a7f4f))
* spread Box props correctly ([9dac265](9dac265)), closes [#190](#190)
* uncontrolled input's label position ([#297](#297)) ([fb6c009](fb6c009))
* window is undefined error in next.js projects ([b89bfc9](b89bfc9)), closes [#222](#222)
@github-actions
Copy link
Contributor

🎉 This PR is included in version 3.0.0-next.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martimalek martimalek martimalek approved these changes

@nlopin nlopin nlopin approved these changes

@div-Leo div-Leo Awaiting requested review from div-Leo

Assignees
No one assigned
Labels
released on @next released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Row component missing children prop type definition
3 participants
@jonotrujillo @nlopin @martimalek