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

Components: Assess stabilization of ItemGroup #61060

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

fullofcaffeine
Copy link
Member

Issue: #59418.

What?

This is part of a larger effort to remove __experimental prefix from all "experimental" components, effectively promoting them to regular stable components. See the related issue for more context.

Why?

The strategy of prefixing exports with __experimental has become deprecated after the introduction of private APIs.

How?

  1. Export it from components without the __experimental prefix;
  2. Keep the old __experimental export for backwards compatibility;
  3. Change all imports of the old __experimental in GB and components to the one without the prefix (including in storybook stories). Also, update the docs to refer to the new unprefixed component;
  4. Add the component storybook id (get it from the storybook URL) to the PREVIOUSLY_EXPERIMENTAL_COMPONENTS const array in manager-head.html so that old experimental story paths are redirected to the new one;
  5. Add a changelog for the change.

@fullofcaffeine fullofcaffeine self-assigned this Apr 25, 2024
@fullofcaffeine fullofcaffeine added [Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality labels Apr 25, 2024
Copy link

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

@fullofcaffeine fullofcaffeine changed the title Components: Remove "experimental" designation for ItemGroup Components: Assess stabilization of ItemGroup Apr 30, 2024
@mirka
Copy link
Member

mirka commented Jul 5, 2024

We should look into how we manage height #63125 (comment)

@ciampo
Copy link
Contributor

ciampo commented Jul 10, 2024

As per #63125 (comment), consensus was reached on:

  • moving to a default size of 40px
  • potentially deprecating the size prop (after carefully considering if we'll ever need a compact size?)
  • treating the 40px height as a minimum height, meaning that children could push <Item /> to be taller. In that case, we'd need to be careful to balance min-height, text line-height and padding to make sure that multiple line text looks good, while also allowing non-text children to display with the right distance from the item borders (padding)

@ciampo ciampo self-assigned this Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants