SlotFill: Migrate to Typescript. #51350
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -65 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
torounit
changed the title
torounit
commented
Jun 8, 2023
torounit
commented
Jun 8, 2023
torounit
commented
Jun 8, 2023
|
Flaky tests detected in ff18ed5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6334034411
|
torounit
changed the title
torounit
commented
Jun 22, 2023
torounit
commented
Jun 22, 2023
torounit
commented
Jun 22, 2023
Comment on lines
79
to
110
| export const WithSlotChildren: ComponentStory< typeof Slot > = ( props ) => { | ||
| return ( | ||
| <SlotFillProvider> | ||
| <h2>Profile</h2> | ||
| <p> | ||
| Name:{ ' ' } | ||
| <Slot { ...props } name="name"> | ||
| { ( fills ) => { | ||
| return ( | ||
| <span style={ { color: 'red' } }>{ fills }</span> | ||
| ); | ||
| } } | ||
| </Slot> | ||
| </p> | ||
| <p> | ||
| Age:{ ' ' } | ||
| <Slot { ...props } name="age"> | ||
| { ( fills ) => { | ||
| return ( | ||
| <span style={ { color: 'red' } }>{ fills }</span> | ||
| ); | ||
| } } | ||
| </Slot> | ||
| </p> | ||
| <Fill name="name">Alice</Fill> | ||
| <Fill name="age">18</Fill> | ||
| </SlotFillProvider> | ||
| ); | ||
| }; | ||
| WithSlotChildren.args = { | ||
| ...Default.args, | ||
| }; |
There was a problem hiding this comment.
Stories when a function is passed to a children of slot. If bubblesVirtually is true, The children are ignored.
torounit
commented
Jun 22, 2023
torounit
commented
Jun 23, 2023
|
Sorry for the delay in reviewing, @torounit . I'll try to take a look, but in the meantime I've also added a few more reviewers. |
tyxla
reviewed
Jun 30, 2023
| ref: MutableRefObject< HTMLElement | undefined >, | ||
| fillProps: any | ||
| ) => void; | ||
| unregisterSlot: ( |
There was a problem hiding this comment.
There are a lot of similarities between the two different context types. Should we try to generalize them a little bit? One way could be with some generic types under the hood.
There was a problem hiding this comment.
That would be neat, although not a blocker IMO. Maybe it can be a timeboxed effort?
packages/components/src/slot-fill/bubbles-virtually/use-slot.ts
Outdated
Show resolved
Hide resolved
packages/components/src/slot-fill/bubbles-virtually/use-slot.ts
Outdated
Show resolved
Hide resolved
packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Marco Ciampini <[email protected]>
torounit
force-pushed
the
refactor/slotfill-bubbles-virtually
branch
from
d42183c to
656a45d
Compare
tyxla
reviewed
Sep 28, 2023
There was a problem hiding this comment.
I think this looks great! It's been a long and complex PR, and I'd lean towards shipping it already.
We can always make some further improvements in subsequent PRs.
I'd prefer to leave the final word to @ciampo though, since he's been involved with many of those TS migrations recently.
Thanks for the great work @torounit 🙌
ciampo
approved these changes
Sep 28, 2023
mikachan
pushed a commit
that referenced
this pull request
Jan 3, 2024
* convert typescript slot-fill-context.ts and slot-fill-provider.tsx * fix portalContainer type. * convert hooks to ts. * fix types * fix fillProps * convert slot.tsx * update Fill * fix typename. * fix dropdown v2 portal container type. * fix ref type * refactor * refactor SlotFillProvider * migrate SlotComponent to TS * convert useSlot * add update * fix ProviderProps * refactor type * refactor type * allow symbol to name prop. * refactor SlotComponentProps * refactor stroies and README * fix typo * remove comments. remove children from Slot. * refactor SlotComponentProps * Apply suggestions from code review Co-authored-by: Lena Morita <[email protected]> * Apply suggestions from code review Co-authored-by: Lena Morita <[email protected]> * refactor: newChildren to inline. * Remove unnecessary type variables. * fix type comments * Remove unnecessary Type Guards. Remove unnecessary Type Guards. And use `React.Key`. * remove type from jsdoc * refactor types * refactor types * Simplify the type of `slot`. * Remove the type specification and leave it to type inference. * fix types * remove story.js * update changelog * remove ts-nocheck * fix story filename. remove override bubblesVirtually attribute * replace useState to useMemo * switch to ts-expect-error in story. * fix mssing changelog https://github.com/WordPress/gutenberg/pull/53272/files/362b6d4405edd476df1f6479bb264dc9f51d6789#r1319926144 * add `children?: never` #51350 (comment) * use Record * use Record / Enable className only when bubblesVirtually: true. * Update packages/components/src/slot-fill/types.ts Co-authored-by: Marco Ciampini <[email protected]> * update changelog * use optional chain * fix WordPressComponentProps import --------- Co-authored-by: Lena Morita <[email protected]> Co-authored-by: Marco Ciampini <[email protected]>
mikachan
pushed a commit
that referenced
this pull request
Jan 5, 2024
* convert typescript slot-fill-context.ts and slot-fill-provider.tsx * fix portalContainer type. * convert hooks to ts. * fix types * fix fillProps * convert slot.tsx * update Fill * fix typename. * fix dropdown v2 portal container type. * fix ref type * refactor * refactor SlotFillProvider * migrate SlotComponent to TS * convert useSlot * add update * fix ProviderProps * refactor type * refactor type * allow symbol to name prop. * refactor SlotComponentProps * refactor stroies and README * fix typo * remove comments. remove children from Slot. * refactor SlotComponentProps * Apply suggestions from code review Co-authored-by: Lena Morita <[email protected]> * Apply suggestions from code review Co-authored-by: Lena Morita <[email protected]> * refactor: newChildren to inline. * Remove unnecessary type variables. * fix type comments * Remove unnecessary Type Guards. Remove unnecessary Type Guards. And use `React.Key`. * remove type from jsdoc * refactor types * refactor types * Simplify the type of `slot`. * Remove the type specification and leave it to type inference. * fix types * remove story.js * update changelog * remove ts-nocheck * fix story filename. remove override bubblesVirtually attribute * replace useState to useMemo * switch to ts-expect-error in story. * fix mssing changelog https://github.com/WordPress/gutenberg/pull/53272/files/362b6d4405edd476df1f6479bb264dc9f51d6789#r1319926144 * add `children?: never` #51350 (comment) * use Record * use Record / Enable className only when bubblesVirtually: true. * Update packages/components/src/slot-fill/types.ts Co-authored-by: Marco Ciampini <[email protected]> * update changelog * use optional chain * fix WordPressComponentProps import --------- Co-authored-by: Lena Morita <[email protected]> Co-authored-by: Marco Ciampini <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
part of #35744. Replaced some class Component with functional components.
Why?
See #35744
How?
Testing Instructions
npm run test:unit -- --testPathPattern packages/components/src/slot-fill/