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.

Sign up for GitHub

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

Refactor LinkControl API #19396

Merged
merged 9 commits into from
Jan 7, 2020
Merged

Refactor LinkControl API #19396

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cd56b75
Refactor LinkControl API
youknowriad Jan 3, 2020
e4a6c23
Fix typos
youknowriad Jan 3, 2020
b0befe3
Simplify popover closing/opening
youknowriad Jan 3, 2020
663f51f
Fix onKeyDown and onKeyPress on the root component
youknowriad Jan 3, 2020
ca03d97
Remove useless onModeChange callback
youknowriad Jan 3, 2020
0b16c6c
Fix new menu item issue
youknowriad Jan 3, 2020
45d1640
Refactor the buttons component
youknowriad Jan 3, 2020
56aa64b
Fix opens in new tab for the buttons block
youknowriad Jan 6, 2020
fff72bc
Move comment to the right place
youknowriad Jan 7, 2020
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
59 changes: 5 additions & 54 deletions packages/block-editor/src/components/link-control/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,26 @@
- Type: `String`
- Required: Yes

### currentLink
### value

- Type: `Object`
- Required: Yes

### currentSettings
### settings

- Type: `Array`
- Required: No
- Default:
```
[
{
id: 'newTab',
id: 'opensInNewTab',
title: 'Open in New Tab',
checked: false,
},
];
```

An array of settings objects. Each object will used to render a `ToggleControl` for that setting. See also `onSettingsChange`.
An array of settings objects. Each object will used to render a `ToggleControl` for that setting.

### fetchSearchSuggestions

Expand All @@ -36,36 +35,12 @@ An array of settings objects. Each object will used to render a `ToggleControl`

## Event handlers

### onChangeMode

- Type: `Function`
- Required: No

Use this callback to know when the LinkControl component changes its mode to `edit` or `show`
through of its function parameter.

```es6
<LinkControl
onChangeMode={ ( mode ) => { console.log( `Mode change to ${ mode } mode.` ) }
/>
```

### onClose

- Type: `Function`
- Required: No

### onKeyDown

- Type: `Function`
- Required: No

### onKeyPress

- Type: `Function`
- Required: No

### onLinkChange
### onChange

- Type: `Function`
- Required: No
Expand All @@ -81,29 +56,5 @@ The function callback will receive the selected item, or Null.
: console.warn( 'No Item selected.' );
}
/>
```

### onSettingsChange

- Type: `Function`
- Required: No
- Args:
- `id` - the `id` property of the setting that changed (eg: `newTab`).
- `value` - the `checked` value of the control.
- `settings` - the current settings object.

Called when any of the settings supplied as `currentSettings` are changed/toggled. May be used to attribute a Block's `attributes` with the current state of the control.

```
<LinkControl
currentSettings={ [
{
id: 'opensInNewTab',
title: __( 'Open in New Tab' ),
checked: attributes.opensInNewTab, // Block attributes persist control state
},
] }
onSettingsChange={ ( setting, value ) => setAttributes( { [ setting ]: value } ) }
/>
```

89 changes: 36 additions & 53 deletions packages/block-editor/src/components/link-control/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { __ } from '@wordpress/i18n';
import {
useCallback,
useState,
useEffect,
Fragment,
} from '@wordpress/element';

Expand All @@ -44,48 +43,33 @@ const MODE_EDIT = 'edit';

function LinkControl( {
className,
currentLink,
currentSettings,
value,
settings,
fetchSearchSuggestions,
instanceId,
onClose = noop,
onChangeMode = noop,
onKeyDown = noop,
onKeyPress = noop,
onLinkChange = noop,
onSettingsChange = noop,
onChange = noop,
} ) {
// State
const [ inputValue, setInputValue ] = useState( '' );
const [ isEditingLink, setIsEditingLink ] = useState( false );

// Effects
useEffect( () => {
getdave marked this conversation as resolved.
Show resolved Hide resolved
// If we have a link then stop editing mode
if ( currentLink ) {
setIsEditingLink( false );
} else {
setIsEditingLink( true );
}
}, [ currentLink ] );
const [ isEditingLink, setIsEditingLink ] = useState( ! value || ! value.url );

// Handlers

/**
* onChange LinkControlSearchInput event handler
*
* @param {string} value Current value returned by the search.
* @param {string} val Current value returned by the search.
*/
const onInputChange = ( value = '' ) => {
setInputValue( value );
const onInputChange = ( val = '' ) => {
setInputValue( val );
};

// Utils

/**
* Handler function which switches the mode of the component,
* between `edit` and `show` mode.
* Also, it calls `onChangeMode` callback function.
*
* @param {string} mode Component mode: `show` or `edit`.
*/
Expand All @@ -94,12 +78,8 @@ function LinkControl( {

// Populate input searcher whether
// the current link has a title.
if ( currentLink && currentLink.title ) {
setInputValue( currentLink.title );
}

if ( isFunction( onChangeMode ) ) {
onChangeMode( mode );
getdave marked this conversation as resolved.
Show resolved Hide resolved
if ( value && value.title && mode === 'edit' ) {
setInputValue( value.title );
getdave marked this conversation as resolved.
Show resolved Hide resolved
}
};

Expand All @@ -112,10 +92,10 @@ function LinkControl( {
setInputValue( '' );
};

const handleDirectEntry = ( value ) => {
const handleDirectEntry = ( val ) => {
Comment on lines -115 to +95
Copy link
Member

Choose a reason for hiding this comment

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

I guess this was changed to avoid shadowing with the top-level prop? I'd typically not ever consider an abbreviation to be an improvement. I might have suggested something like nextValue if that's what it's intended to represent in this context.

let type = 'URL';

const protocol = getProtocol( value ) || '';
const protocol = getProtocol( val ) || '';

if ( protocol.includes( 'mailto' ) ) {
type = 'mailto';
Expand All @@ -125,27 +105,27 @@ function LinkControl( {
type = 'tel';
}

if ( startsWith( value, '#' ) ) {
if ( startsWith( val, '#' ) ) {
type = 'internal';
}

return Promise.resolve(
[ {
id: '-1',
title: value,
url: type === 'URL' ? prependHTTP( value ) : value,
title: val,
url: type === 'URL' ? prependHTTP( val ) : val,
type,
} ]
);
};

const handleEntitySearch = async ( value ) => {
const handleEntitySearch = async ( val ) => {
const results = await Promise.all( [
fetchSearchSuggestions( value ),
handleDirectEntry( value ),
fetchSearchSuggestions( val ),
handleDirectEntry( val ),
] );

const couldBeURL = ! value.includes( ' ' );
const couldBeURL = ! val.includes( ' ' );

// If it's potentially a URL search then concat on a URL search suggestion
// just for good measure. That way once the actual results run out we always
Expand All @@ -154,15 +134,15 @@ function LinkControl( {
};

// Effects
const getSearchHandler = useCallback( ( value ) => {
const protocol = getProtocol( value ) || '';
const getSearchHandler = useCallback( ( val ) => {
const protocol = getProtocol( val ) || '';
const isMailto = protocol.includes( 'mailto' );
const isInternal = startsWith( value, '#' );
const isInternal = startsWith( val, '#' );
const isTel = protocol.includes( 'tel' );

const handleManualEntry = isInternal || isMailto || isTel || isURL( value ) || ( value && value.includes( 'www.' ) );
const handleManualEntry = isInternal || isMailto || isTel || isURL( val ) || ( val && val.includes( 'www.' ) );

return ( handleManualEntry ) ? handleDirectEntry( value ) : handleEntitySearch( value );
return ( handleManualEntry ) ? handleDirectEntry( val ) : handleEntitySearch( val );
}, [ handleDirectEntry, fetchSearchSuggestions ] );

// Render Components
Expand All @@ -181,7 +161,10 @@ function LinkControl( {
key={ `${ suggestion.id }-${ suggestion.type }` }
itemProps={ buildSuggestionItemProps( suggestion, index ) }
suggestion={ suggestion }
onClick={ () => onLinkChange( suggestion ) }
onClick={ () => {
setIsEditingLink( false );
getdave marked this conversation as resolved.
Show resolved Hide resolved
onChange( { ...value, ...suggestion } );
} }
isSelected={ index === selectedSuggestion }
isURL={ manualLinkEntryTypes.includes( suggestion.type.toLowerCase() ) }
searchTerm={ inputValue }
Expand All @@ -202,7 +185,7 @@ function LinkControl( {
<div className="block-editor-link-control__popover-inner">
<div className="block-editor-link-control__search">

{ ( ! isEditingLink && currentLink ) && (
{ ( ! isEditingLink ) && (
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't have a link then we don't want to show this UI interface. Just curious as to why you feel it's safe to remove this now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the boolean seemed explicit enough for me. If we do add the check for the value here, we should also add it to force showing the input if the value is empty which was not the case.

<Fragment>
<p className="screen-reader-text" id={ `current-link-label-${ instanceId }` }>
{ __( 'Currently selected' ) }:
Expand All @@ -215,14 +198,13 @@ function LinkControl( {
} ) }
>
<span className="block-editor-link-control__search-item-header">

<ExternalLink
className="block-editor-link-control__search-item-title"
href={ currentLink.url }
href={ value.url }
>
{ currentLink.title }
{ value.title }
</ExternalLink>
<span className="block-editor-link-control__search-item-info">{ filterURLForDisplay( safeDecodeURI( currentLink.url ) ) || '' }</span>
<span className="block-editor-link-control__search-item-info">{ filterURLForDisplay( safeDecodeURI( value.url ) ) || '' }</span>
</span>

<Button isSecondary onClick={ setMode( MODE_EDIT ) } className="block-editor-link-control__search-item-action block-editor-link-control__search-item-action--edit">
Expand All @@ -236,17 +218,18 @@ function LinkControl( {
<LinkControlSearchInput
value={ inputValue }
onChange={ onInputChange }
onSelect={ onLinkChange }
onSelect={ ( suggestion ) => {
setIsEditingLink( false );
onChange( { ...value, ...suggestion } );
} }
renderSuggestions={ renderSearchResults }
fetchSuggestions={ getSearchHandler }
onReset={ resetInput }
onKeyDown={ onKeyDown }
onKeyPress={ onKeyPress }
/>
) }

{ ! isEditingLink && (
<LinkControlSettingsDrawer settings={ currentSettings } onSettingChange={ onSettingsChange } />
<LinkControlSettingsDrawer value={ value } settings={ settings } onChange={ onChange } />
getdave marked this conversation as resolved.
Show resolved Hide resolved
) }
</div>
</div>
Expand Down
27 changes: 22 additions & 5 deletions packages/block-editor/src/components/link-control/search-input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,39 @@
*/
import { __ } from '@wordpress/i18n';
import { Button } from '@wordpress/components';
import { ENTER } from '@wordpress/keycodes';
import { LEFT,
RIGHT,
UP,
DOWN,
BACKSPACE,
ENTER,
} from '@wordpress/keycodes';

/**
* Internal dependencies
*/
import { URLInput } from '../';

const handleLinkControlOnKeyDown = ( event ) => {
getdave marked this conversation as resolved.
Show resolved Hide resolved
const { keyCode } = event;

if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( keyCode ) > -1 ) {
// Stop the key event from propagating up to ObserveTyping.startTypingInTextField.
event.stopPropagation();
}
};

const handleLinkControlOnKeyPress = ( event ) => {
event.stopPropagation();
};

const LinkControlSearchInput = ( {
value,
onChange,
onSelect,
renderSuggestions,
fetchSuggestions,
onReset,
onKeyDown,
onKeyPress,
} ) => {
const selectItemHandler = ( selection, suggestion ) => {
onChange( selection );
Expand All @@ -44,9 +61,9 @@ const LinkControlSearchInput = ( {
if ( event.keyCode === ENTER ) {
return;
}
onKeyDown( event );
handleLinkControlOnKeyDown( event );
} }
onKeyPress={ onKeyPress }
onKeyPress={ handleLinkControlOnKeyPress }
placeholder={ __( 'Search or type url' ) }
__experimentalRenderSuggestions={ renderSuggestions }
__experimentalFetchLinkSuggestions={ fetchSuggestions }
Expand Down
Loading