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

Add Missing URL state to Navigation Link Block #28861

Merged
merged 19 commits into from
Feb 26, 2021
Merged
6 changes: 5 additions & 1 deletion packages/block-editor/src/components/link-control/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,17 @@ $block-editor-link-control-number-of-actions: 1;
}

.block-editor-link-control__settings {
border-top: 1px solid $gray-300;
border-top: $border-width solid $gray-300;
margin: 0;
padding: $grid-unit-20 $grid-unit-30;

:last-child {
margin-bottom: 0;
}

.is-alternate & {
border-top: $border-width solid $gray-900;
}
}

.block-editor-link-control__setting {
Expand Down
3 changes: 3 additions & 0 deletions packages/block-editor/src/components/url-input/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ class URLInput extends Component {
}

componentWillUnmount() {
if ( this.suggestionsRequest?.cancel ) {
this.suggestionsRequest.cancel();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found this during debugging, if you quickly toggle the URLInput before the fetch returns, it will attempt to call setState() on the unmounted component. This cancels the suggestionsRequest Promise when it unmounts to avoid the issue.

I can break it out to a separate PR if needed, the rest of the code doesn't trigger this anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably okay to sneak in, maybe just leave a quick note in the summary. eg "also fixes ..."

Minor: We can remove the if check using optional chaining.

this?.suggestionsRequest?.cancel?.()

See also: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining#stacking_the_optional_chaining_operator

delete this.suggestionsRequest;
}

Expand Down
103 changes: 76 additions & 27 deletions packages/block-library/src/navigation-link/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ export default function NavigationLinkEdit( {
},
} );

if ( ! url ) {
blockProps.onClick = () => setIsLinkOpen( true );
}

const innerBlocksProps = useInnerBlocksProps(
{
className: classnames( 'wp-block-navigation__container', {
Expand All @@ -332,6 +336,28 @@ export default function NavigationLinkEdit( {
}
);

const classes = classnames( 'wp-block-navigation-link__content', {
'wp-block-navigation-link__placeholder': ! url,
} );

let missingText = '';
switch ( type ) {
case 'post':
missingText = __( 'Select a post' );
break;
case 'page':
missingText = __( 'Select a page' );
break;
case 'category':
missingText = __( 'Select a category' );
break;
case 'tag':
missingText = __( 'Select a tag' );
break;
default:
missingText = __( 'Add a link' );
georgeh marked this conversation as resolved.
Show resolved Hide resolved
}

return (
<Fragment>
<BlockControls>
Expand Down Expand Up @@ -389,38 +415,61 @@ export default function NavigationLinkEdit( {
</PanelBody>
</InspectorControls>
<li { ...blockProps }>
<div className="wp-block-navigation-link__content">
<RichText
ref={ ref }
identifier="label"
className="wp-block-navigation-link__label"
value={ label }
onChange={ ( labelValue ) =>
setAttributes( { label: labelValue } )
}
onMerge={ mergeBlocks }
onReplace={ onReplace }
__unstableOnSplitAtEnd={ () =>
insertBlocksAfter(
createBlock( 'core/navigation-link' )
)
}
aria-label={ __( 'Navigation link text' ) }
placeholder={ itemLabelPlaceholder }
keepPlaceholderOnFocus
withoutInteractiveFormatting
allowedFormats={ [
'core/bold',
'core/italic',
'core/image',
'core/strikethrough',
] }
/>
<div className={ classes }>
{ ! url ? (
<div className="wp-block-navigation-link__placeholder-text">
<KeyboardShortcuts
shortcuts={ {
enter: () =>
isSelected && setIsLinkOpen( true ),
} }
/>
{ missingText }
</div>
) : (
<RichText
ref={ ref }
identifier="label"
className="wp-block-navigation-link__label"
value={ label }
onChange={ ( labelValue ) =>
setAttributes( { label: labelValue } )
}
onMerge={ mergeBlocks }
onReplace={ onReplace }
__unstableOnSplitAtEnd={ () =>
insertBlocksAfter(
createBlock( 'core/navigation-link' )
)
}
aria-label={ __( 'Navigation link text' ) }
placeholder={ itemLabelPlaceholder }
keepPlaceholderOnFocus
withoutInteractiveFormatting
allowedFormats={ [
'core/bold',
'core/italic',
'core/image',
'core/strikethrough',
] }
onClick={ () => {
if ( ! url ) {
setIsLinkOpen( true );
}
} }
/>
) }
{ isLinkOpen && (
<Popover
position="bottom center"
onClose={ () => setIsLinkOpen( false ) }
>
<KeyboardShortcuts
bindGlobal
shortcuts={ {
escape: () => setIsLinkOpen( false ),
} }
/>
<LinkControl
georgeh marked this conversation as resolved.
Show resolved Hide resolved
className="wp-block-navigation-link__inline-link-input"
value={ link }
Expand Down
44 changes: 44 additions & 0 deletions packages/block-library/src/navigation-link/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,47 @@
display: none;
}
}

// Menu item setup state, is shown when a menu item has no URL configured.
.wp-block-navigation-link__placeholder {
position: relative;
cursor: pointer;

// Provide a little margin to show each placeholder as a separate unit.
margin: 2px;

.wp-block-navigation-link__placeholder-text {
font-family: $default-font;
font-size: $default-font-size;
}

&::before {
content: "";
display: block;
position: absolute;
top: 0;
right: 0;
bottom: 0;
left: 0;
border-radius: $radius-block-ui;
opacity: 0.1;

.is-dark-theme & {
opacity: 0.2;
}

.is-editing & {
background: currentColor;
}
}
}

// We had to add extra classes to override the color from
// .editor-styles-wrapper .wp-block-navigation:not(.has-text-color) .wp-block-navigation-link__content
.wp-block-navigation
.wp-block-navigation-link:not(.is-editing)
.wp-block-navigation-link__content.wp-block-navigation-link__placeholder {
box-shadow: inset 0 0 0 $border-width $gray-700;
border-radius: $radius-block-ui;
color: var(--wp-admin-theme-color);
}