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.

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

Tabs: Update subcomponents to accept full HTML element props #55860

Merged
merged 5 commits into from Nov 7, 2023

Conversation

chad1008
Copy link
Contributor

@chad1008 chad1008 commented Nov 3, 2023

Related comment.

What?

Updates the subcomponents (TabList, Tab, and TabPanel) to accept all of the HTML element props relevant to their underlying DOM elements.

This PR also removes some artifacts of an old icon prop that was removed earlier in the component's development. There were still some references in the types and tests files that needed to be removed.

Why?

Previously the Tabs subcomponents explicitly accepted props like className and style, but not the general props for their respective DOM elements. This made it hard to pass basic props, and made it necessary to overuse the render prop to gain access to them (example)

How?

Each of the subcomponents now leverages WordPressComponentProps to include all of the relevant HTML element props.

Testing Instructions

  1. Open the Tabs default story in Storybook.
  2. Edit the default story by adding some div related HTML props to a TabList and TabPanel subcomponent (hidden, style, className for example, but experiment with any prop that's valid on a div.)
  3. Confirm that the props you've added are correctly passed to the rendered div for the subcomponent you modified.
  4. Edit the default story by adding some button related props to a Tab subcomponent. Specifically, test an onClick to confirm that it's applied and functions correctly.
  5. Have a snack. You deserve it.

@chad1008 chad1008 added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Nov 3, 2023
@chad1008 chad1008 self-assigned this Nov 3, 2023
Copy link
Contributor

@brookewp brookewp 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 testing well! ✅

Since props are being removed (className and style), I wonder if it would be beneficial to add any tests? But since this is utilizing WordPressComponentProps, it may be unnecessary. I'll leave that up to your discretion! Otherwise, I believe this just needs a CHANGELOG, and then it'll be ready to go.

@chad1008
Copy link
Contributor Author

chad1008 commented Nov 6, 2023

Thanks @brookewp! I think I'll hold off on tests now... I feel like I'd be writing tests for WordPressComponentProps more than for Tabs itself - but I'm happy to add them in a followup later if we decide it's warranted!

Copy link

github-actions bot commented Nov 6, 2023

Flaky tests detected in 98094ee.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6788313246
📝 Reported issues:

@chad1008 chad1008 merged commit 1b15cda into trunk Nov 7, 2023
50 checks passed
@chad1008 chad1008 deleted the tabs-html-props branch November 7, 2023 18:41
@github-actions github-actions bot added this to the Gutenberg 17.1 milestone Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants