Skip to content

Commit

Permalink
eslint: Enable @wordpress/no-base-control-with-label-without-id rule (
Browse files Browse the repository at this point in the history
#39708)

When `<BaseControl>` is passed both a `label` and `id`, it makes an HTML
`<label>` so clicking the label focuses the contained element with the
corresponding `id`. Leaving off the `id` breaks that, so this rule flags
the situation for fixing.

It turns out, though, that everything it flagged in the monorepo was a
false positive.

A few cases we had something like
`<BaseControl label><TextControl /></BaseControl>`. There's no reason
for that when we can do `<TextControl label />` instead and get the
expected click-label-to-focus behavior.

The rest of the cases we were using `<BaseControl label>` to get a label
for something more complex, where there's really no `id` that makes
sense. In this case the documented thing to do is apparently
`<BaseControl><BaseControl.VisualLabel>label</BaseControl.VisualLabel></BaseControl>`
which does exactly the same thing that `<BaseControl label>` with no
`id` does internally.
  • Loading branch information
anomiex authored and gogdzl committed Oct 25, 2024
1 parent 3a8b034 commit 3efdf01
Show file tree
Hide file tree
Showing 23 changed files with 95 additions and 80 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fixed
Comment: Clean up `<BaseControl label>` without `id`. Should be no change in functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ export default function GeneratedImagePreview( {

return (
<ThemeProvider>
<BaseControl
__nextHasNoMarginBottom={ true }
label={ _x( 'Preview', 'Heading for the generated preview image', 'jetpack' ) }
>
<BaseControl __nextHasNoMarginBottom={ true }>
<BaseControl.VisualLabel>
{ _x( 'Preview', 'Heading for the generated preview image', 'jetpack' ) }
</BaseControl.VisualLabel>
<div className={ styles.container }>
<img
className={ clsx( {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,8 @@ export default function MediaSection( {

return (
<ThemeProvider>
<BaseControl
__nextHasNoMarginBottom={ true }
label={ __( 'Attached Media', 'jetpack' ) }
className={ styles.wrapper }
>
<BaseControl __nextHasNoMarginBottom={ true } className={ styles.wrapper }>
<BaseControl.VisualLabel>{ __( 'Attached Media', 'jetpack' ) }</BaseControl.VisualLabel>
{ renderHeaderSection() }
<MediaWrapper { ...mediaWrapperProps }>
<MediaPicker
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,8 @@ const SocialImageGeneratorSettingsModal = ( { onClose } ) => {
'jetpack'
) }
/>
<BaseControl
__nextHasNoMarginBottom={ true }
label={ __( 'Templates', 'jetpack' ) }
className={ styles.templateControl }
>
<BaseControl __nextHasNoMarginBottom={ true } className={ styles.templateControl }>
<BaseControl.VisualLabel>{ __( 'Templates', 'jetpack' ) }</BaseControl.VisualLabel>
<TemplatePicker value={ localTemplate } onTemplateSelected={ setEditedTemplate } />
</BaseControl>
<Button onClick={ onClose } variant="tertiary">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fixed
Comment: Clean up `<BaseControl label>` without `id`. Should be no change in functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import { __ } from '@wordpress/i18n';
export default function JetpackFieldWidth( { setAttributes, width } ) {
return (
<BaseControl
label={ __( 'Field Width', 'jetpack-forms' ) }
help={ __(
'Adjust the width of the field to include multiple fields on a single line.',
'jetpack-forms'
) }
className="jetpack-field-label__width"
>
<BaseControl.VisualLabel>{ __( 'Field Width', 'jetpack-forms' ) }</BaseControl.VisualLabel>
<ButtonGroup aria-label={ __( 'Field Width', 'jetpack-forms' ) }>
{ [ 25, 50, 75, 100 ].map( widthValue => {
return (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fixed
Comment: Clean up `<BaseControl label>` without `id`. Should be no change in functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ const UploadingEditor = props => {

return (
<div className="uploading-editor">
<BaseControl label={ __( 'Video poster (optional)', 'jetpack-videopress-pkg' ) }>
<BaseControl>
<BaseControl.VisualLabel>
{ __( 'Video poster (optional)', 'jetpack-videopress-pkg' ) }
</BaseControl.VisualLabel>
<Poster
file={ file }
videoPosterImageData={ videoPosterImageData }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: other
Comment: Clean up `<BaseControl label>` without `id`. Should be no change in functionality, other than in a few cases being able to click the label to focus the field now.


Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ function FeedbackControl() {

return (
<div className="jetpack-ai-feedback-control">
<BaseControl label={ __( 'Feedback', 'jetpack' ) }>
<BaseControl>
<BaseControl.VisualLabel>{ __( 'Feedback', 'jetpack' ) }</BaseControl.VisualLabel>
<p>
{ __(
'Your feedback is valuable in our commitment to refine and improve this feature.',
Expand Down
29 changes: 10 additions & 19 deletions projects/plugins/jetpack/extensions/blocks/ai-chat/controls.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import { InspectorAdvancedControls, InspectorControls } from '@wordpress/block-editor';
import {
BaseControl,
PanelBody,
TextControl,
TextareaControl,
ToggleControl,
} from '@wordpress/components';
import { PanelBody, TextControl, TextareaControl, ToggleControl } from '@wordpress/components';
import { useEntityProp } from '@wordpress/core-data';
import { __ } from '@wordpress/i18n';
import { DEFAULT_PLACEHOLDER } from './constants';
Expand All @@ -26,28 +20,25 @@ export function AiChatControls( {
<>
<InspectorControls>
<PanelBody title={ __( 'Settings', 'jetpack' ) } initialOpen={ false }>
<BaseControl
<TextControl
label={ __( 'Placeholder Text', 'jetpack' ) }
className="jetpack-ai-chat__ask-button-text"
>
<TextControl
placeholder={ DEFAULT_PLACEHOLDER }
onChange={ newPlaceholder => setAttributes( { placeholder: newPlaceholder } ) }
value={ placeholder }
/>
</BaseControl>
placeholder={ DEFAULT_PLACEHOLDER }
onChange={ newPlaceholder => setAttributes( { placeholder: newPlaceholder } ) }
value={ placeholder }
/>
</PanelBody>
</InspectorControls>
<InspectorAdvancedControls>
<BaseControl
<TextareaControl
label={ __( 'Additional instructions', 'jetpack' ) }
help={ __(
'Give Jetpack AI additional instructions for answer length, format, and tone.',
'jetpack'
) }
>
<TextareaControl value={ promptOverride } onChange={ setPromptOverride } />
</BaseControl>
value={ promptOverride }
onChange={ setPromptOverride }
/>
<ToggleControl
label={ __( 'Show copy answer button.', 'jetpack' ) }
help={ __( 'Allow users to easily copy the answer.', 'jetpack' ) }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BaseControl, TextControl } from '@wordpress/components';
import { TextControl } from '@wordpress/components';
import { Component, createRef } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import Lookup from '../lookup';
Expand Down Expand Up @@ -74,10 +74,11 @@ export class MapboxLocationSearch extends Component {
const { label } = this.props;
return (
<div ref={ this.containerRef }>
<BaseControl label={ label } className="components-location-search">
<div className="components-location-search">
<Lookup completer={ this.autocompleter } onReset={ this.onReset }>
{ ( { isExpanded, listBoxId, activeId, onChange, onKeyDown } ) => (
<TextControl
label={ label }
placeholder={ placeholderText }
ref={ this.textRef }
onChange={ onChange }
Expand All @@ -88,7 +89,7 @@ export class MapboxLocationSearch extends Component {
/>
) }
</Lookup>
</BaseControl>
</div>
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BaseControl, TextControl } from '@wordpress/components';
import { TextControl } from '@wordpress/components';
import { useEffect, useRef } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import Lookup from '../lookup';
Expand Down Expand Up @@ -88,10 +88,11 @@ const MapkitLocationSearch = ( { label, onAddPoint } ) => {

return (
<div ref={ containerRef }>
<BaseControl label={ label } className="components-location-search">
<div className="components-location-search">
<Lookup completer={ autocompleter } onReset={ onReset }>
{ ( { isExpanded, listBoxId, activeId, onChange, onKeyDown } ) => (
<TextControl
label={ label }
placeholder={ placeholderText }
ref={ textRef }
onChange={ onChange }
Expand All @@ -102,7 +103,7 @@ const MapkitLocationSearch = ( { label, onAddPoint } ) => {
/>
) }
</Lookup>
</BaseControl>
</div>
</div>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ export default function WhatsAppButtonConfiguration( { attributes, setAttributes
<>
<BaseControl
__nextHasNoMarginBottom={ true }
label={ __( 'Phone Number', 'jetpack' ) }
help={ __(
'Enter the phone number you use for WhatsApp and would like to be contacted on.',
'jetpack'
) }
className="jetpack-whatsapp-button__phonenumber"
>
<BaseControl.VisualLabel>{ __( 'Phone Number', 'jetpack' ) }</BaseControl.VisualLabel>
<SelectControl
__nextHasNoMarginBottom={ true }
label={ __( 'Country code', 'jetpack' ) }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,21 @@
import { BaseControl, PanelBody, TextControl } from '@wordpress/components';
import { PanelBody, TextControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

export function PanelControls( { setAttributes, postLinkText } ) {
return (
<PanelBody title={ __( 'Settings', 'jetpack' ) } initialOpen={ false }>
<BaseControl
<TextControl
__nextHasNoMarginBottom={ true }
label={ __( 'Purchase link text', 'jetpack' ) }
help={ __(
'Enter the text you want to display on a purchase link used as fallback when the PayPal button cannot be used (e.g. emails, AMP, etc.)',
'jetpack'
) }
className="jetpack-simple-payments__purchase-link-text"
>
<TextControl
__nextHasNoMarginBottom={ true }
placeholder={ __( 'Click here to purchase', 'jetpack' ) }
onChange={ newPostLinkText => setAttributes( { postLinkText: newPostLinkText } ) }
value={ postLinkText }
/>
</BaseControl>
placeholder={ __( 'Click here to purchase', 'jetpack' ) }
onChange={ newPostLinkText => setAttributes( { postLinkText: newPostLinkText } ) }
value={ postLinkText }
/>
</PanelBody>
);
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { getCurrencyDefaults } from '@automattic/format-currency';
import { InspectorControls } from '@wordpress/block-editor';
import {
BaseControl,
Disabled,
ExternalLink,
PanelBody,
Expand Down Expand Up @@ -412,22 +411,19 @@ class SimplePaymentsEdit extends Component {
renderSettings = () => (
<InspectorControls>
<PanelBody title={ __( 'Settings', 'jetpack' ) } initialOpen={ false }>
<BaseControl
<TextControl
label={ __( 'Purchase link text', 'jetpack' ) }
help={ __(
'Enter the text you want to display on a purchase link used as fallback when the PayPal button cannot be used (e.g. emails, AMP, etc.)',
'jetpack'
) }
className="jetpack-simple-payments__purchase-link-text"
>
<TextControl
placeholder={ __( 'Click here to purchase', 'jetpack' ) }
onChange={ newPostLinkText =>
this.props.setAttributes( { postLinkText: newPostLinkText } )
}
value={ this.props.attributes.postLinkText }
/>
</BaseControl>
placeholder={ __( 'Click here to purchase', 'jetpack' ) }
onChange={ newPostLinkText =>
this.props.setAttributes( { postLinkText: newPostLinkText } )
}
value={ this.props.attributes.postLinkText }
/>
</PanelBody>
</InspectorControls>
);
Expand Down
8 changes: 4 additions & 4 deletions projects/plugins/jetpack/extensions/blocks/videopress/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -567,10 +567,10 @@ const VideoPressEdit = CoreVideoEdit =>
help={ this.getPreloadHelp() }
/>
<MediaUploadCheck>
<BaseControl
className="editor-video-poster-control"
label={ __( 'Poster Image', 'jetpack' ) }
>
<BaseControl className="editor-video-poster-control">
<BaseControl.VisualLabel>
{ __( 'Poster Image', 'jetpack' ) }
</BaseControl.VisualLabel>
<MediaUpload
title={ __( 'Select Poster Image', 'jetpack' ) }
onSelect={ this.onSelectPoster }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ export const UploadingEditor = props => {
value={ title }
/>
<div className="uploading-editor__content">
<BaseControl label={ __( 'Video poster (optional)', 'jetpack' ) }>
<BaseControl>
<BaseControl.VisualLabel>
{ __( 'Video poster (optional)', 'jetpack' ) }
</BaseControl.VisualLabel>
{ canDisplayThumbnailScrubber ? (
<>
<div className="uploading-editor__video-container">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,28 +90,36 @@ const JetpackAndSettingsContent = ( {

{ canWriteBriefBeEnabled() && isBreveAvailable && (
<PanelRow>
<BaseControl label={ __( 'Write Brief with AI (BETA)', 'jetpack' ) }>
<BaseControl>
<BaseControl.VisualLabel>
{ __( 'Write Brief with AI (BETA)', 'jetpack' ) }
</BaseControl.VisualLabel>
<Breve />
</BaseControl>
</PanelRow>
) }

<PanelRow className="jetpack-ai-sidebar__feature-section">
<BaseControl label={ __( 'AI Feedback', 'jetpack' ) }>
<BaseControl>
<BaseControl.VisualLabel>{ __( 'AI Feedback', 'jetpack' ) }</BaseControl.VisualLabel>
<Feedback placement={ placement } busy={ false } disabled={ requireUpgrade } />
</BaseControl>
</PanelRow>

{ isAITitleOptimizationAvailable && (
<PanelRow className="jetpack-ai-sidebar__feature-section">
<BaseControl label={ titleOptimizationSectionLabel }>
<BaseControl>
<BaseControl.VisualLabel>{ titleOptimizationSectionLabel }</BaseControl.VisualLabel>
<TitleOptimization placement={ placement } busy={ false } disabled={ requireUpgrade } />
</BaseControl>
</PanelRow>
) }
{ isAIFeaturedImageAvailable && (
<PanelRow className="jetpack-ai-sidebar__feature-section">
<BaseControl label={ __( 'AI Featured Image', 'jetpack' ) }>
<BaseControl>
<BaseControl.VisualLabel>
{ __( 'AI Featured Image', 'jetpack' ) }
</BaseControl.VisualLabel>
<FeaturedImage busy={ false } disabled={ requireUpgrade } placement={ placement } />
</BaseControl>
</PanelRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ function UsageControl( {
);

return (
<BaseControl help={ help } label={ __( 'Available Requests', 'jetpack' ) }>
<BaseControl help={ help }>
<BaseControl.VisualLabel>{ __( 'Available Requests', 'jetpack' ) }</BaseControl.VisualLabel>
{ ! loading && usageDisplay }
{ loading && loadingPlaceholder }
</BaseControl>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,8 @@ export function AiExcerptControl( {

return (
<div className="jetpack-ai-generate-excerpt-control">
<BaseControl
className="jetpack-ai-generate-excerpt-control__header"
label={ __( 'Settings', 'jetpack' ) }
>
<BaseControl className="jetpack-ai-generate-excerpt-control__header">
<BaseControl.VisualLabel>{ __( 'Settings', 'jetpack' ) }</BaseControl.VisualLabel>
<Button
label={ __( 'Advanced AI options', 'jetpack' ) }
icon={ aiAssistantIcon }
Expand Down
5 changes: 4 additions & 1 deletion projects/plugins/jetpack/extensions/shared/width-panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ export function WidthControl( { align, width, onChange, showLabel = true } ) {
}

return (
<BaseControl __nextHasNoMarginBottom={ true } label={ showLabel && __( 'Width', 'jetpack' ) }>
<BaseControl __nextHasNoMarginBottom={ true }>
{ showLabel && (
<BaseControl.VisualLabel>{ __( 'Width', 'jetpack' ) }</BaseControl.VisualLabel>
) }
<div
className={ clsx( 'jetpack-block-width-controls', {
'is-aligned': isAlignedLeftOrRight,
Expand Down
Loading

0 comments on commit 3efdf01

Please sign in to comment.