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: Remove unnecessary stories #68463

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Tabs: Remove unnecessary stories #68463

merged 1 commit into from
Jan 2, 2025

Conversation

mirka
Copy link
Member

@mirka mirka commented Jan 2, 2025

Prerequisite for #68329

What?

Removes unnecessary Storybook stories for the Tabs component:

  • Controlled mode
  • Tab getting removed dynamically
  • Tab getting disabled dynamically

Why?

Every story should have value to our audience, because each story adds noise and maintenance cost.

The "Size And Overflow Playground" story is basically a test fixture and should also eventually be removed or at least suppressed from the Storybook. I am leaving it for now because I assume these overflow features are still somewhat under development (cc @ciampo).

Testing Instructions

See Storybook for Tabs.

@mirka mirka added [Type] Developer Documentation Documentation for developers [Package] Components /packages/components Storybook Storybook and its stories for components labels Jan 2, 2025
@mirka mirka self-assigned this Jan 2, 2025
@mirka mirka requested a review from ajitbohra as a code owner January 2, 2025 12:28
Copy link

github-actions bot commented Jan 2, 2025

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: mirka <[email protected]>
Co-authored-by: tyxla <[email protected]>

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

@@ -367,133 +366,3 @@ const CloseButtonTemplate: StoryFn< typeof Tabs > = ( props ) => {
);
};
export const InsertCustomElements = CloseButtonTemplate.bind( {} );

const ControlledModeTemplate: StoryFn< typeof Tabs > = ( props ) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Controlled mode is explained better on the Best Practices page. Behavior should/can be covered in the unit tests.

selectedTabId: 'tab3',
};

const TabBecomesDisabledTemplate: StoryFn< typeof Tabs > = ( props ) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Behavior should/can be covered in the unit tests.

};
export const TabBecomesDisabled = TabBecomesDisabledTemplate.bind( {} );

const TabGetsRemovedTemplate: StoryFn< typeof Tabs > = ( props ) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Behavior should/can be covered in the unit tests.

@mirka mirka requested a review from a team January 2, 2025 12:38
Copy link

github-actions bot commented Jan 2, 2025

Flaky tests detected in eeab286.
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/12582426057
📝 Reported issues:

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Should we move those to unit tests before completely obliterating them?

@mirka
Copy link
Member Author

mirka commented Jan 2, 2025

Should we move those to unit tests before completely obliterating them?

My wording was a bit ambiguous 🙈 I meant that the unit tests already cover them.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Just confirmed that this is the case.

Let's clean them stories up then 🚀

@mirka mirka merged commit 979c44d into trunk Jan 2, 2025
73 of 74 checks passed
@mirka mirka deleted the remove-tabs-stories branch January 2, 2025 17:52
@github-actions github-actions bot added this to the Gutenberg 20.1 milestone Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components Storybook Storybook and its stories for components [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants