Refactor Dropdown component to TypeScript #45787

kienstra · 2022-11-15T23:14:28Z

What?

Convert the Dropdown component to TypeScript

Why?

As part of an effort to convert @wordpress/components to TypeScript

How?

Mainly by adding types to the Dropdown component

Testing Instructions

npm run storybook:dev
Expected: The Storybook still works

Screenshots

codesandbox · 2022-11-15T23:14:30Z

Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

packages/components/src/border-control/border-control-dropdown/component.tsx

Might be the wrong approach.

kienstra · 2022-12-08T02:17:39Z

packages/components/src/border-control/border-control-dropdown/component.tsx

@@ -165,7 +165,9 @@ const BorderControlDropdown = (
 		? 'bottom left'
 		: undefined;

-	const renderToggle = ( { onToggle = noop } ) => (
+	const renderToggle: DropdownComponentProps[ 'renderToggle' ] = ( {
+		onToggle,


renderToggle always has an onToggle prop, so no need for the default of noop.

kienstra · 2022-12-08T02:20:17Z

packages/components/src/dropdown/README.md

 -   Required: No

-### position
+### `expandOnMobile`: `boolean`


The props are now alphabetized, so this diff is a little hard to read.

kienstra · 2022-12-08T02:21:15Z

packages/components/src/dropdown/index.tsx

+function useObservableState(
+	initialState: boolean,
+	onStateChange?: ( newState: boolean ) => void
+): [ boolean, ( newState: boolean ) => void ] {


You'd hope that TS would infer the return type from the return statement below, but it doesn't.

return [ state, + ( value: boolean ) => { setState( value ); if ( onStateChange ) { onStateChange( value ); } }, + ] as const;

☝️ The trick is to append an as const here ✨

Whoa, thanks! So much cleaner to let TS infer the type.

Committed in 1f9ff85

kienstra · 2022-12-08T02:23:45Z

packages/components/src/dropdown/index.tsx

@@ -70,8 +77,13 @@ export default function Dropdown( props ) {
 	 * case a dialog has opened, allowing focus to return when it's dismissed.
 	 */
 	function closeIfFocusOutside() {
+		if ( ! containerRef.current ) {
+			return;


This is hacky, but without it there are TS errors:

FWIW I don't think this is hacky. It's a proper guard 🙂 I love early returns!

kienstra · 2022-12-08T02:29:03Z

packages/components/src/dropdown/stories/index.tsx

@@ -21,20 +27,27 @@ export default {
 		renderContent: { control: { type: null } },
 		renderToggle: { control: { type: null } },
 	},
+	parameters: {


Without parameters, there's an error with npm run storybook:build:

ERR! => Failed to build the preview ERR! Module build failed (from ./storybook/webpack/source-link-loader.js): ERR! TypeError: wp-content/plugins/gutenberg/packages/components/src/dropdown/stories/index.tsx: Property value of ObjectProperty expected node to be of a type ["Expression","PatternLike"] but instead got undefined

Ah, you caught a bug, thank you! I prepped a fix in #46670.

Ah, thanks for finding that!

kienstra · 2022-12-08T02:34:56Z

packages/components/src/dropdown/index.tsx

+ * );
+ * ```
+ */
+export const Dropdown = forwardRef( UnforwardedDropdown );


Maybe I did this wrong.

But <Dropdown> is passed a ref prop here.

So the Dropdown needs to be wrapped in forwardRef() to accept that ref.

Good catch 👍 Looks good to me.

kienstra · 2022-12-08T02:36:52Z

packages/components/src/dropdown/test/index.tsx

@@ -24,14 +24,13 @@ describe( 'Dropdown', () => {
 					</button>
 				) }
 				renderContent={ () => <span>test</span> }
-				popoverProps={ { 'data-testid': 'popover' } }


This caused a TS error:

Huh, interesting! Looks like other component libraries are also experiencing this issue.
mui/material-ui#20160 (comment)

Oh, good to see we're not the only ones to find that problem

kienstra · 2022-12-08T02:39:58Z

packages/components/src/dropdown/test/index.tsx

 			/>
 		);

 		const button = screen.getByRole( 'button', { expanded: false } );

 		expect( button ).toBeVisible();
-		expect( screen.queryByTestId( 'popover' ) ).not.toBeInTheDocument();
+		expect( screen.queryByText( 'test' ) ).not.toBeInTheDocument();


Changed because popoverProps can't accept { 'data-testid': 'popover' }.

Instead of doing a @ts-ignore for that type error, I changed this.

This is testing the same thing as before: whether <Popover> is rendering.

If <Popover> is rendering, it will call renderContent(), which will render <span>test</span>.

Good call. Works for me 👍

kienstra · 2022-12-08T04:20:41Z

Hi @ciampo,
Could you use this PR?

Hope you're doing well! It's been a while 😄

mirka

Sorry this went unnoticed for a while, @ciampo has been away on vacation for a few weeks!

Anyway, awesome work on this one 🎉 I really appreciate your attention to detail, and the code comments you left made it easy to review.

FYI you already meet the criteria to join the Gutenberg team, which will make life easier for you when you contribute, like push branches directly to the repo, edit issues, or merge your own PRs once they're approved. See the "Teams" section in this doc if you'd like to join 🙌

mirka · 2022-12-20T10:30:19Z

packages/components/src/dropdown/stories/index.tsx

@@ -21,20 +27,27 @@ export default {
 		renderContent: { control: { type: null } },
 		renderToggle: { control: { type: null } },
 	},
+	parameters: {


Ah, you caught a bug, thank you! I prepped a fix in #46670.

packages/components/src/dropdown/stories/index.tsx

mirka · 2022-12-20T12:03:11Z

packages/components/src/dropdown/index.tsx

+function useObservableState(
+	initialState: boolean,
+	onStateChange?: ( newState: boolean ) => void
+): [ boolean, ( newState: boolean ) => void ] {


return [ state, + ( value: boolean ) => { setState( value ); if ( onStateChange ) { onStateChange( value ); } }, + ] as const;

☝️ The trick is to append an as const here ✨

packages/components/src/dropdown/index.tsx

mirka · 2022-12-20T13:51:50Z

packages/components/src/color-palette/index.tsx

@@ -134,7 +135,7 @@ export function CustomColorPickerDropdown( {
 	popoverProps: receivedPopoverProps,
 	...props
 }: CustomColorPickerDropdownProps ) {
-	const popoverProps = useMemo(
+	const popoverProps = useMemo< DropdownProps[ 'popoverProps' ] >(


Looks good to me 👍

mirka · 2022-12-20T13:54:22Z

packages/components/src/color-palette/types.ts

@@ -37,15 +37,8 @@ export type MultiplePalettesProps = PaletteProps & {
 	colors: PaletteObject[];
 };

-// TODO: should extend `Dropdown`'s props once it gets refactored to TypeScript


packages/components/src/dropdown/README.md

packages/components/src/dropdown/types.ts

…own/index.tsx Co-authored-by: Lena Morita <lena@jaguchi.com>

…own/stories/index.tsx Co-authored-by: Lena Morita <lena@jaguchi.com>

…own/index.tsx Co-authored-by: Lena Morita <lena@jaguchi.com>

…Props

…own/types.ts Co-authored-by: Lena Morita <lena@jaguchi.com>

…own/README.md Co-authored-by: Lena Morita <lena@jaguchi.com>

kienstra · 2022-12-20T20:16:52Z

Hi @mirka,
Thanks for your great ideas, the as const one is really nice.

I committed all of your suggestions.

Also, thanks for your idea of joining the GitHub Gutenberg team. I just requested that in #core-editor.

mirka

Really great work here, thank you! Looking forward to see more of your contributions 😄

kienstra · 2022-12-21T15:42:25Z

Thanks so much, Lena! You had really good ideas, like typeof Popover.

Refactor Dropdown component to TypeScript

a5eee07

kienstra added 15 commits November 15, 2022 17:27

Correct the text of fail()

1198f14

Rename RenderProps, as those have a different meaning

710a0b5

Rename EmbeddedComponentProps to CallbackProps

8f2301c

Merge in trunk, resolve conflicts in text.js

1e4ad45

Fix failed unit test

e6fdf90

Correct the text to search for

de50e19

Correct the popoverProps property

6bde338

Simplify the type of popoverProps

500c295

Add types to story exports

16adebe

Revert changes to index.native.js

d16f15b

Delete index.native.tsx

baec628

Merge branch 'trunk' into update/dropdown-to-ts

a7b0bf5

Merge branch 'trunk' into update/dropdown-to-ts

ced6b08

Use the CallbackProps from types.ts

4fe9498

Pass parameters to stories so there's not a TypeError

f44af7a

kienstra commented Nov 30, 2022

View reviewed changes

packages/components/src/border-control/border-control-dropdown/component.tsx Outdated Show resolved Hide resolved

kienstra added 3 commits November 29, 2022 22:18

Reference the DropdownProps instead of definitin the props again

41b86a6

Move a type import to the other type imports

0069fa3

Change the Dropdown to accept a forwarded ref

b68677e

Might be the wrong approach.

kienstra commented Dec 8, 2022

View reviewed changes

Don't export CallbackProps

c56dc5c

kienstra commented Dec 8, 2022

View reviewed changes

mirka mentioned this pull request Dec 13, 2022

Refactor @wordpress/components package to TypeScript #35744

Closed

mirka assigned kienstra Dec 13, 2022

mirka added the [Package] Components /packages/components label Dec 13, 2022

mirka requested review from ciampo and mirka December 13, 2022 18:50

mirka reviewed Dec 20, 2022

View reviewed changes

mirka mentioned this pull request Dec 20, 2022

Storybook: Fix crash when parameters is undefined #46670

Merged

kienstra and others added 11 commits December 20, 2022 12:27

Commit Lena Morita's suggestion: Update packages/components/src/dropd…

82bbf50

…own/index.tsx Co-authored-by: Lena Morita <lena@jaguchi.com>

Commit Lena Morita's suggestion: Update packages/components/src/dropd…

775f10b

…own/stories/index.tsx Co-authored-by: Lena Morita <lena@jaguchi.com>

Apply Lena Morita's suggestion to let TS infer the return type

1f9ff85

Commit Lena Morita's suggestion: Update packages/components/src/dropd…

22f699e

…own/index.tsx Co-authored-by: Lena Morita <lena@jaguchi.com>

Define className and style, instead of referencing WordPressComponent…

181cfda

…Props

Remove expandOnMobile and headerTitle in stories

0ebe8ce

Commit Lena Morita's suggestion: Update packages/components/src/dropd…

33c6715

…own/types.ts Co-authored-by: Lena Morita <lena@jaguchi.com>

Commit Lena Morita's suggestion: Update packages/components/src/dropd…

7595fa5

…own/types.ts Co-authored-by: Lena Morita <lena@jaguchi.com>

Apply Lena Morita's suggestion use typeof Popover

e693603

Commit Lena Morita's suggestion: Update packages/components/src/dropd…

94640d3

…own/README.md Co-authored-by: Lena Morita <lena@jaguchi.com>

Merge in trunk, resolve conflicts

319cdd9

mirka approved these changes Dec 21, 2022

View reviewed changes

mirka merged commit 79f6e5f into WordPress:trunk Dec 21, 2022

github-actions bot added this to the Gutenberg 14.9 milestone Dec 21, 2022

kienstra deleted the update/dropdown-to-ts branch December 21, 2022 15:41

kienstra mentioned this pull request Dec 22, 2022

Refactor QueryControls component to TypeScript #46721

Merged

ciampo mentioned this pull request Jan 3, 2023

Dropdown: the position prop is made redundant by popoverProps #46858

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Dropdown component to TypeScript #45787

Refactor Dropdown component to TypeScript #45787

kienstra commented Nov 15, 2022 •

edited

Loading

codesandbox bot commented Nov 15, 2022

kienstra Dec 8, 2022 •

edited

Loading

kienstra Dec 8, 2022

kienstra Dec 8, 2022

mirka Dec 20, 2022

kienstra Dec 20, 2022

kienstra Dec 8, 2022

mirka Dec 20, 2022

kienstra Dec 20, 2022

kienstra Dec 8, 2022

mirka Dec 20, 2022

kienstra Dec 20, 2022

kienstra Dec 8, 2022 •

edited

Loading

mirka Dec 20, 2022

kienstra Dec 20, 2022

kienstra Dec 8, 2022

mirka Dec 20, 2022

kienstra Dec 20, 2022 •

edited

Loading

kienstra Dec 8, 2022 •

edited

Loading

mirka Dec 20, 2022

kienstra commented Dec 8, 2022

mirka left a comment

mirka Dec 20, 2022

mirka Dec 20, 2022

mirka Dec 20, 2022

mirka Dec 20, 2022

kienstra commented Dec 20, 2022 •

edited

Loading

mirka left a comment

kienstra commented Dec 21, 2022 •

edited

Loading

Refactor Dropdown component to TypeScript #45787

Refactor Dropdown component to TypeScript #45787

Conversation

kienstra commented Nov 15, 2022 • edited Loading

What?

Why?

How?

Testing Instructions

Screenshots

codesandbox bot commented Nov 15, 2022

kienstra Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra Dec 20, 2022 • edited Loading

Choose a reason for hiding this comment

kienstra Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra commented Dec 8, 2022

mirka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra commented Dec 20, 2022 • edited Loading

mirka left a comment

Choose a reason for hiding this comment

kienstra commented Dec 21, 2022 • edited Loading

kienstra commented Nov 15, 2022 •

edited

Loading

kienstra Dec 8, 2022 •

edited

Loading

kienstra Dec 8, 2022 •

edited

Loading

kienstra Dec 20, 2022 •

edited

Loading

kienstra Dec 8, 2022 •

edited

Loading

kienstra commented Dec 20, 2022 •

edited

Loading

kienstra commented Dec 21, 2022 •

edited

Loading