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

SearchControl: refactor to use InputControl internally #56524

Merged
merged 36 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
0cbc074
WIP
ciampo Nov 25, 2023
bb34bad
Merge branch 'trunk' into refactor/search-control-input-control
mirka Dec 27, 2023
fcf93e8
Confirm label working as expected
mirka Dec 27, 2023
a59328f
Simplify id
mirka Dec 27, 2023
91c0399
Simplify props
mirka Dec 27, 2023
1c3e0a1
Fix types
mirka Dec 27, 2023
58952b2
Add `__nextHasNoMarginBottom` back compat
mirka Dec 27, 2023
1b56b19
Add RTL todo
mirka Dec 27, 2023
c7bdd22
Add CSS classes
mirka Dec 28, 2023
2863577
Adjust prefix/suffix
mirka Dec 28, 2023
c10bb7a
Add empty string fallback to `value`
mirka Dec 28, 2023
57d1b5e
Add placeholder
mirka Dec 28, 2023
a2e6f71
Reset webkit styles
mirka Dec 28, 2023
44616b7
Restore original onChange type signature
mirka Dec 28, 2023
2933784
Merge branch 'trunk' into refactor/search-control-input-control
mirka Jan 19, 2024
f579ab6
Make 40px by default
mirka Jan 19, 2024
63851ec
Support compact size
mirka Jan 19, 2024
061474c
Adjust inner padding
mirka Jan 19, 2024
619df49
Adjust close button size
mirka Jan 19, 2024
0c845c0
Remove stylesheet
mirka Jan 19, 2024
26efc70
Tweak JSDocs
mirka Jan 19, 2024
e078c86
Add default label
mirka Jan 19, 2024
0ac7a31
Tweak story
mirka Jan 19, 2024
785bd56
Clean up
mirka Jan 19, 2024
baf8f3c
Update docs
mirka Jan 19, 2024
93a2b4c
Purify SuffixItem
mirka Feb 5, 2024
552175b
Tighten icon margins
mirka Feb 5, 2024
3c3797c
Merge branch 'trunk' into refactor/search-control-input-control
mirka Feb 5, 2024
e91440c
Support borderless gray
mirka Feb 6, 2024
9c18f1e
Restore original icon layout
mirka Feb 6, 2024
cf5bb5c
Merge branch 'trunk' into refactor/search-control-input-control
mirka Feb 7, 2024
aa2d573
Programatically remove any potential `disabled` prop
mirka Feb 7, 2024
62b49c0
Align `value` fallback to null coalesce
mirka Feb 8, 2024
5247c85
Add GH handle for design team
mirka Feb 8, 2024
377cb5c
Add changelog
mirka Feb 8, 2024
adb0b1f
Update SearchControl consumers (#58014)
mirka Feb 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions packages/block-editor/src/components/inserter/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,6 @@ $block-inserter-tabs-height: 44px;

.block-editor-inserter__search {
padding: $grid-unit-20 $grid-unit-20 0 $grid-unit-20;

.components-search-control__icon {
right: $grid-unit-10 + ($grid-unit-60 - $icon-size) * 0.5;
}
}

.block-editor-inserter__tabs {
Expand Down Expand Up @@ -615,15 +611,9 @@ $block-inserter-tabs-height: 44px;
}

.block-editor-inserter__media-panel-search {
&.components-search-control {
input[type="search"].components-search-control__input {
background: $white;
}
button.components-button {
min-width: auto;
padding-left: $grid-unit-05 * 0.5;
padding-right: $grid-unit-05 * 0.5;
}
// TODO: Consider using the Theme component to automatically adapt to a gray background.
&:not(:focus-within) {
--wp-components-color-background: #{$white};
}
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
- Removing Reakit as a dependency of the components package ([#58631](https://github.com/WordPress/gutenberg/pull/58631)).
- `CustomSelect`: add unit tests ([#58583](https://github.com/WordPress/gutenberg/pull/58583)).
- `InputBase`: Add `isBorderless` prop ([#58750](https://github.com/WordPress/gutenberg/pull/58750)).
- `SearchControl`: Replace internal implementation to use `InputControl` ([#56524](https://github.com/WordPress/gutenberg/pull/56524)).

## 25.16.0 (2024-01-24)

Expand Down
12 changes: 6 additions & 6 deletions packages/components/src/navigation/menu/menu-title-search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import { __, _n, sprintf } from '@wordpress/i18n';
import withSpokenMessages from '../../higher-order/with-spoken-messages';
import { useNavigationMenuContext } from './context';
import { useNavigationContext } from '../context';
import { MenuTitleSearchUI } from '../styles/navigation-styles';
import { SEARCH_FOCUS_DELAY } from '../constants';

import type { NavigationMenuTitleSearchProps } from '../types';
import SearchControl from '../../search-control';
import { MenuTitleSearchControlWrapper } from '../styles/navigation-styles';

function MenuTitleSearch( {
debouncedSpeak,
Expand Down Expand Up @@ -80,20 +81,19 @@ function MenuTitleSearch( {
).trim();

return (
<div className="components-navigation__menu-title-search">
<MenuTitleSearchUI
autoComplete="off"
<MenuTitleSearchControlWrapper>
<SearchControl
__nextHasNoMarginBottom
className="components-navigation__menu-search-input"
id={ inputId }
onChange={ ( value ) => onSearch?.( value ) }
onKeyDown={ onKeyDown }
placeholder={ placeholder }
onClose={ onClose }
ref={ inputRef }
type="search"
value={ search }
/>
</div>
</MenuTitleSearchControlWrapper>
);
}

Expand Down
32 changes: 5 additions & 27 deletions packages/components/src/navigation/styles/navigation-styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { Text } from '../../text';
import { Heading } from '../../heading';
import { reduceMotion, rtl } from '../../utils';
import { space } from '../../utils/space';
import SearchControl from '../../search-control';

export const NavigationUI = styled.div`
width: 100%;
Expand Down Expand Up @@ -69,6 +68,11 @@ export const MenuTitleUI = styled.div`
width: 100%;
`;

export const MenuTitleSearchControlWrapper = styled.div`
margin: 11px 0; // non-ideal hardcoding to maintain same height as Heading, could be improved
padding: 1px; // so the focus border doesn't get cut off by the overflow hidden on MenuTitleUI
`;

export const MenuTitleActionsUI = styled.span`
height: ${ space( 6 ) }; // 24px, same height as the buttons inside

Expand All @@ -91,32 +95,6 @@ export const MenuTitleActionsUI = styled.span`
}
`;

export const MenuTitleSearchUI = styled( SearchControl )`
input[type='search'].components-search-control__input {
margin: 0;
background: #303030;
color: #fff;

&:focus {
background: #434343;
color: #fff;
}

&::placeholder {
color: rgba( 255, 255, 255, 0.6 );
}
}

svg {
fill: white;
}

.components-button.has-icon {
padding: 0;
min-width: auto;
}
`;

export const GroupTitleUI = styled( Heading )`
min-height: ${ space( 12 ) };
align-items: center;
Expand Down
26 changes: 23 additions & 3 deletions packages/components/src/search-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ function MySearchControl( { className, setState } ) {

return (
<SearchControl
__nextHasNoMarginBottom
label={ __( 'Search posts' ) }
value={ searchInput }
onChange={ setSearchInput }
Expand All @@ -35,13 +36,14 @@ Props not included in this set will be applied to the input element.

#### label

If this property is added, a label will be generated using label property as the content.
The accessible label for the input.

A label should always be provided as an accessibility best practice, even when a placeholder is defined
and `hideLabelFromVision` is `true`.

- Type: `String`
- Required: Yes
- Required: No
- Default: `__( 'Search' )`

#### placeholder

Expand Down Expand Up @@ -72,19 +74,37 @@ A function that receives the value of the input.
- Type: `function`
- Required: Yes

#### onClose

When an `onClose` callback is provided, the search control will render a close button that will trigger the given callback.

Use this if you want the button to trigger your own logic to close the search field entirely, rather than just clearing the input value.

- Type: `function`
- Required: No

#### help

If this property is added, a help text will be generated using help property as the content.

- Type: `String|Element`
- Required: No

### hideLabelFromVision
#### hideLabelFromVision

If true, the label will not be visible, but will be read by screen readers. Defaults to `true`.

- Type: `Boolean`
- Required: No
- Default: `true`

#### __nextHasNoMarginBottom

Start opting into the new margin-free styles that will become the default in a future version.

- Type: `Boolean`
- Required: No
- Default: `false`

#### `size`: `'default'` | `'compact'`

Expand Down
158 changes: 88 additions & 70 deletions packages/components/src/search-control/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,100 +9,117 @@ import classnames from 'classnames';
import { useInstanceId, useMergeRefs } from '@wordpress/compose';
import { __ } from '@wordpress/i18n';
import { Icon, search, closeSmall } from '@wordpress/icons';
import { forwardRef, useRef } from '@wordpress/element';
import { forwardRef, useMemo, useRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import Button from '../button';
import BaseControl from '../base-control';
import type { WordPressComponentProps } from '../context/wordpress-component';
import type { SearchControlProps } from './types';
import type { SearchControlProps, SuffixItemProps } from './types';
import type { ForwardedRef } from 'react';
import { ContextSystemProvider } from '../context';
import { StyledInputControl, SuffixItemWrapper } from './styles';

function SuffixItem( {
searchRef,
value,
onChange,
onClose,
}: SuffixItemProps ) {
if ( ! onClose && ! value ) {
return <Icon icon={ search } />;
}

const onReset = () => {
onChange( '' );
searchRef.current?.focus();
};

return (
<Button
size="small"
icon={ closeSmall }
label={ onClose ? __( 'Close search' ) : __( 'Reset search' ) }
onClick={ onClose ?? onReset }
/>
);
}

function UnforwardedSearchControl(
{
__nextHasNoMarginBottom,
__next40pxDefaultSize = false,
__nextHasNoMarginBottom = false,
className,
onChange,
onKeyDown,
value,
label,
label = __( 'Search' ),
Copy link
Member

Choose a reason for hiding this comment

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

I noticed that this component didn't have a default label, and since it was not marked as required in TS and visually hidden by default, there were likely a lot of unlabeled usages 😔 I gave it a default and updated the docs.

placeholder = __( 'Search' ),
hideLabelFromVision = true,
help,
onClose,
size = 'default',
...restProps
}: WordPressComponentProps< SearchControlProps, 'input', false >,
}: Omit<
WordPressComponentProps< SearchControlProps, 'input', false >,
// TODO: Background styling currently doesn't support a disabled state. Needs design work.
'disabled'
>,
forwardedRef: ForwardedRef< HTMLInputElement >
) {
const searchRef = useRef< HTMLInputElement >();
const instanceId = useInstanceId( SearchControl );
const id = `components-search-control-${ instanceId }`;

const renderRightButton = () => {
if ( onClose ) {
return (
<Button
__next40pxDefaultSize={ __next40pxDefaultSize }
icon={ closeSmall }
label={ __( 'Close search' ) }
onClick={ onClose }
size={ size }
/>
);
}
// @ts-expect-error The `disabled` prop is not yet supported in the SearchControl component.
// Work with the design team (@WordPress/gutenberg-design) if you need this feature.
delete restProps.disabled;

if ( !! value ) {
return (
<Button
__next40pxDefaultSize={ __next40pxDefaultSize }
icon={ closeSmall }
label={ __( 'Reset search' ) }
onClick={ () => {
onChange( '' );
searchRef.current?.focus();
} }
size={ size }
/>
);
}
const searchRef = useRef< HTMLInputElement >( null );
const instanceId = useInstanceId(
SearchControl,
'components-search-control'
);

return <Icon icon={ search } />;
};
const contextValue = useMemo(
() => ( {
// Overrides the underlying BaseControl `__nextHasNoMarginBottom` via the context system
// to provide backwards compatibile margin for SearchControl.
// (In a standard InputControl, the BaseControl `__nextHasNoMarginBottom` is always set to true.)
BaseControl: { _overrides: { __nextHasNoMarginBottom } },
// `isBorderless` is still experimental and not a public prop for InputControl yet.
InputBase: { isBorderless: true },
} ),
[ __nextHasNoMarginBottom ]
);

return (
<BaseControl
__nextHasNoMarginBottom={ __nextHasNoMarginBottom }
label={ label }
id={ id }
hideLabelFromVision={ hideLabelFromVision }
help={ help }
className={ classnames( className, 'components-search-control', {
'is-next-40px-default-size': __next40pxDefaultSize,
'is-size-compact': size === 'compact',
} ) }
>
<div className="components-search-control__input-wrapper">
<input
{ ...restProps }
ref={ useMergeRefs( [ searchRef, forwardedRef ] ) }
className="components-search-control__input"
id={ id }
type="search"
placeholder={ placeholder }
onChange={ ( event ) => onChange( event.target.value ) }
onKeyDown={ onKeyDown }
autoComplete="off"
value={ value || '' }
/>
<div className="components-search-control__icon">
{ renderRightButton() }
</div>
</div>
</BaseControl>
<ContextSystemProvider value={ contextValue }>
<StyledInputControl
__next40pxDefaultSize
id={ instanceId }
hideLabelFromVision={ hideLabelFromVision }
label={ label }
ref={ useMergeRefs( [ searchRef, forwardedRef ] ) }
type="search"
size={ size }
className={ classnames(
'components-search-control',
className
) }
onChange={ ( nextValue?: string ) =>
onChange( nextValue ?? '' )
Copy link
Member

Choose a reason for hiding this comment

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

Nit: sometimes we can see value || '' and sometimes value ?? ''. While for a normal input control, there shouldn't be a big difference, perhaps we can make these consistent.

Copy link
Member

Choose a reason for hiding this comment

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

Good idea, I'll unify to ?? since they're closer to the intent. The only value that it would affect here is an empty string, and since we're falling back to an empty string it shouldn't change the outcome. 62b49c0

}
autoComplete="off"
placeholder={ placeholder }
value={ value ?? '' }
suffix={
<SuffixItemWrapper size={ size }>
<SuffixItem
searchRef={ searchRef }
value={ value }
onChange={ onChange }
onClose={ onClose }
/>
</SuffixItemWrapper>
}
{ ...restProps }
/>
</ContextSystemProvider>
);
}

Expand All @@ -118,6 +135,7 @@ function UnforwardedSearchControl(
*
* return (
* <SearchControl
* __nextHasNoMarginBottom
* value={ searchInput }
* onChange={ setSearchInput }
* />
Expand Down
Loading
Loading