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

BaseControl: Deprecate bottom margin #38730

Closed
49 tasks done
Tracked by #47771
mirka opened this issue Feb 11, 2022 · 2 comments · Fixed by #64408
Closed
49 tasks done
Tracked by #47771

BaseControl: Deprecate bottom margin #38730

mirka opened this issue Feb 11, 2022 · 2 comments · Fixed by #64408
Assignees
Labels
Needs Dev Note Requires a developer note for a major WordPress release cycle [Package] Components /packages/components [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@mirka
Copy link
Member

mirka commented Feb 11, 2022

What problem does this address?

The BaseControl component currently has an inherent margin-bottom, making it hard to reuse in different contexts, since it requires a style override of an internal element. This kind of internal override is something we want to discourage.

Additionally, the current margin-bottom leads to inconsistent results when help text is supplied, because the margin is inserted before and not after the help text.

What is your proposed solution?

Deprecate the bottom margin (similar to #37160), following the guidelines stipulated in the devdocs.

Because this change impacts a large number of components (even just inside the Gutenberg codebase), I suggest we approach this in phases:

✍️ Dev Note (WordPress 6.2 release)

A number of UI components currently ship with styles that give them top and/or bottom margins. This can make it hard to reuse them in arbitrary layouts, where you want different amounts of gap or margin between components.

To better suit modern layout needs, we will gradually deprecate these outer margins. A deprecation will begin with an opt-in period where you can choose to apply the new margin-free styles on a given component instance. Eventually in a future version, the margins will be completely removed.

Continuing the effort started in previous releases, for the WordPress 6.2 release we focused on the BaseControl component and its consumers. While the effort is still ongoing, the outer margins on the following components have been deprecated.

  • BaseControl
  • CheckboxControl
  • ComboboxControl
  • FocalPointPicker
  • RadioControl
  • RangeControl
  • SearchControl
  • SelectControl
  • TextControl
  • TextareaControl
  • ToggleControl
  • ToggleGroupControl
  • TreeSelect

To start opting into the new margin-free styles, set the __nextHasNoMarginBottom prop to true.

<SelectControl
  options={ selectOptions }
  value={ selectValue }
  label={ __( 'Label' ) }
  onChange={ onSelectChange }
  __nextHasNoMarginBottom={ true }
/>

We're in the process of migrating all internal usages to use the new __nextHasNoMarginBottom prop. Once all usages are migrated, an official deprecation will be added to the BaseControl component

@ciampo
Copy link
Contributor

ciampo commented May 24, 2022

Just flagging that, with #41269, the __nextHasNoMarginBottom prop was added to the SelectControl component.

@ciampo
Copy link
Contributor

ciampo commented Apr 17, 2023

igorschoester added a commit to Yoast/wordpress-seo that referenced this issue Sep 12, 2024
Gutenberg added a deprecation warning for SelectControl via WordPress/gutenberg#38730
This opts in to that behavior and adds the margin top to the ExternalLink below (therefor needing block layout to get the margin to take effect)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Note Requires a developer note for a major WordPress release cycle [Package] Components /packages/components [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
Status: Done 🎉
Development

Successfully merging a pull request may close this issue.

4 participants