Skip to content

Fix: Expose TypeScript types for subcomponents #69619

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
Merged
merged 16 commits into from
Apr 8, 2025
Merged

Fix: Expose TypeScript types for subcomponents #69619

merged 16 commits into from
Apr 8, 2025

Conversation

Rishit30G
Copy link
Contributor

@Rishit30G Rishit30G commented Mar 19, 2025 •
edited
Loading

What?

Closes #69615

Why?

This PR solves the issue of the subcomponents of the Popover component not exposing their TypeScript type

How?

Just like the Menu, types for the subcomponents of the Popover are also exposed and no longer require @ts-expect-error

Testing Instructions

N/A

Screenshots or screencast

N/A

@Mamaduka Mamaduka added [Type] Developer Documentation Documentation for developers Developer Experience Ideas about improving block and theme developer experience labels Mar 19, 2025
@Rishit30G Rishit30G marked this pull request as ready for review March 19, 2025 06:58
@Rishit30G Rishit30G requested a review from ajitbohra as a code owner March 19, 2025 06:58
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 19, 2025 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Rishit30G <rishit30g@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@t-hamano t-hamano added the [Package] Components /packages/components label Mar 19, 2025
mirka
mirka reviewed Mar 19, 2025
View reviewed changes
@ciampo
Copy link
Contributor

ciampo commented Mar 20, 2025

Refactoring all compound components to use the Object.assign syntax would be great — probably best to have a tracking issue to coordinate the effort, and then tackle each component one by one?

mirka
mirka reviewed Mar 21, 2025
View reviewed changes
Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

Looking good!

We also want to list Popover.Slot in the Storybook subcomponents. For the props table to work, this will require a named export for the PopoverSlot function before Object.assigning it. So something like:

export const PopoverSlot = forwardRef(
	( { name = SLOT_NAME }: { name?: string }, ref: ForwardedRef< any > ) => {
		return (
			<Slot
				bubblesVirtually
				name={ name }
				className="popover-slot"
				ref={ ref }
			/>
		);
	}
);

@Rishit30G
Copy link
Contributor Author

Thanks for the review @mirka!

I have added the PopoverSlot subcomponent, however I'm not sure how to and what to add in the prop description for PopoverSlot

@Rishit30G Rishit30G requested a review from mirka March 23, 2025 08:11
@ciampo
Copy link
Contributor

ciampo commented Mar 24, 2025

@Rishit30G you can take inspiration from existing compound components such as Tabs

@Rishit30G
Copy link
Contributor Author

Thanks @ciampo! 🙇🏻

I have updated the PR accordingly, sharing a screenshot of the Popover.Slot storybook:

image

@Rishit30G Rishit30G requested a review from ciampo March 25, 2025 03:40
ciampo
ciampo reviewed Mar 28, 2025
View reviewed changes
@Rishit30G Rishit30G requested a review from ciampo March 29, 2025 07:12
ciampo
ciampo reviewed Apr 3, 2025
View reviewed changes
ciampo
ciampo reviewed Apr 3, 2025
View reviewed changes
@Rishit30G Rishit30G requested a review from ciampo April 7, 2025 05:30
ciampo
ciampo reviewed Apr 8, 2025
View reviewed changes
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

This is looking good, code-wise! We just need to add a CHANGELOG entry

@Rishit30G Rishit30G requested a review from ciampo April 8, 2025 13:31
@Rishit30G
Copy link
Contributor Author

Sure, I have added the entry 👍🏻

ciampo
ciampo reviewed Apr 8, 2025
View reviewed changes
@Rishit30G Rishit30G requested a review from ciampo April 8, 2025 14:19
ciampo
ciampo approved these changes Apr 8, 2025
View reviewed changes
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

ciampo
ciampo reviewed Apr 8, 2025
View reviewed changes
ciampo
ciampo reviewed Apr 8, 2025
View reviewed changes
@ciampo ciampo enabled auto-merge (squash) April 8, 2025 14:25
ciampo
ciampo reviewed Apr 8, 2025
View reviewed changes
@ciampo ciampo merged commit b17d1ca into WordPress:trunk Apr 8, 2025
59 checks passed
@github-actions github-actions bot added this to the Gutenberg 20.7 milestone Apr 8, 2025
chriszarate pushed a commit to chriszarate/gutenberg that referenced this pull request Jul 1, 2025
@Rishit30G @ciampo
@mirka @chriszarate
Fix: Expose TypeScript types for subcomponents (WordPress#69619)
a33288b 
* Update popover subcomponents

* Remove internal usage note from Popover.Slot documentation

* Add PopoverSlot component to Popover and update stories

* Enhance PopoverSlot component with TypeScript props and update documentation

* Refactor PopoverSlot component props

* Remove comment

* add PopoverSlot as named export

* Add changelog entry

* Add fullstop

* Update documentation section

* Update changelog documentation

* Update letter casing

* Update packages/components/CHANGELOG.md

* Update packages/components/CHANGELOG.md

---------

Co-authored-by: Rishit30G <rishit30g@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ciampo ciampo ciampo approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@mirka mirka Awaiting requested review from mirka

Assignees
No one assigned
Labels
Developer Experience Ideas about improving block and theme developer experience [Package] Components /packages/components [Type] Developer Documentation Documentation for developers
Projects
None yet
Milestone
Gutenberg 20.7
Development

Successfully merging this pull request may close these issues.

Popover: Expose TypeScript types for subcomponents
5 participants
@Rishit30G @ciampo @mirka @Mamaduka @t-hamano