-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Changes from 32 commits
0cbc074
bb34bad
fcf93e8
a59328f
91c0399
1c3e0a1
58952b2
1b56b19
c7bdd22
2863577
c10bb7a
57d1b5e
a2e6f71
44616b7
2933784
f579ab6
63851ec
061474c
619df49
0c845c0
26efc70
e078c86
0ac7a31
785bd56
baf8f3c
93a2b4c
552175b
3c3797c
e91440c
9c18f1e
cf5bb5c
aa2d573
62b49c0
5247c85
377cb5c
adb0b1f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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' ), | ||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should mention the team handle? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice 👍 5247c85 |
||
delete restProps.disabled; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this little safe guard so consumers don't improvise a |
||
|
||
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 ?? '' ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: sometimes we can see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, I'll unify to |
||
} | ||
autoComplete="off" | ||
placeholder={ placeholder } | ||
value={ value || '' } | ||
suffix={ | ||
<SuffixItemWrapper size={ size }> | ||
<SuffixItem | ||
searchRef={ searchRef } | ||
value={ value } | ||
onChange={ onChange } | ||
onClose={ onClose } | ||
/> | ||
</SuffixItemWrapper> | ||
} | ||
{ ...restProps } | ||
/> | ||
</ContextSystemProvider> | ||
); | ||
} | ||
|
||
|
@@ -118,6 +135,7 @@ function UnforwardedSearchControl( | |
* | ||
* return ( | ||
* <SearchControl | ||
* __nextHasNoMarginBottom | ||
* value={ searchInput } | ||
* onChange={ setSearchInput } | ||
* /> | ||
|
This file was deleted.
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 ] }; | ||
} | ||
`; |
There was a problem hiding this comment.
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.