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 32 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
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 if you need this feature.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should mention the team handle? @WordPress/gutenberg-design?

Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍 5247c85

delete restProps.disabled;
Copy link
Member

Choose a reason for hiding this comment

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

I added this little safe guard so consumers don't improvise a disabled variant of the SearchControl. Currently, the light gray background of the search box is exactly the same color as a disabled InputControl, so we're going to need some coordinated design work if anyone ever needs a disabled search box.


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
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const meta: Meta< typeof SearchControl > = {
component: SearchControl,
argTypes: {
onChange: { action: 'onChange' },
value: { control: { type: null } },
},
parameters: {
controls: { expanded: true },
Expand Down Expand Up @@ -46,7 +47,6 @@ const Template: StoryFn< typeof SearchControl > = ( {

export const Default = Template.bind( {} );
Default.args = {
label: 'Label Text',
help: 'Help text to explain the input.',
};

Expand Down
71 changes: 0 additions & 71 deletions packages/components/src/search-control/style.scss

This file was deleted.

42 changes: 42 additions & 0 deletions packages/components/src/search-control/styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* External dependencies
*/
import styled from '@emotion/styled';

/**
* Internal dependencies
*/
import { space } from '../utils/space';
import InputControl from '../input-control';
import { COLORS } from '../utils';
import type { SearchControlProps } from './types';

const inlinePadding = ( {
size,
}: Required< Pick< SearchControlProps, 'size' > > ) => {
return space( size === 'compact' ? 1 : 2 );
};

export const SuffixItemWrapper = styled.div`
display: flex;
padding-inline-end: ${ inlinePadding };

svg {
fill: currentColor;
}
`;

export const StyledInputControl = styled( InputControl )`
input[type='search'] {
&::-webkit-search-decoration,
&::-webkit-search-cancel-button,
&::-webkit-search-results-button,
&::-webkit-search-results-decoration {
-webkit-appearance: none;
}
}

&:not( :focus-within ) {
--wp-components-color-background: ${ COLORS.theme.gray[ 100 ] };
}
`;
Loading
Loading