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

Fix Link UI when hyperlink has an empty href value #35043

Merged
merged 5 commits into from
Sep 30, 2021
+85 −15
Merged

Fix Link UI when hyperlink has an empty href value #35043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7fb7f45
Add empty variation to the Link Preview
getdave Sep 21, 2021
15dadc7
Switch to using "error" to describe failed state
getdave Sep 22, 2021
6e3e20e
Add test coverage for empty link state
getdave Sep 22, 2021
1207a4f
Add context via comments
getdave Sep 22, 2021
a1e8f94
Use info icon as per designers direction
getdave Sep 23, 2021
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
51 changes: 36 additions & 15 deletions packages/block-editor/src/components/link-control/link-preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
__experimentalText as Text,
} from '@wordpress/components';
import { filterURLForDisplay, safeDecodeURI } from '@wordpress/url';
import { Icon, globe } from '@wordpress/icons';
import { Icon, globe, info } from '@wordpress/icons';

/**
* Internal dependencies
Expand All @@ -38,6 +38,19 @@ export default function LinkPreview( {
const displayURL =
( value && filterURLForDisplay( safeDecodeURI( value.url ), 16 ) ) ||
'';

const isEmptyURL = ! value.url.length;

let icon;

if ( richData?.icon ) {
icon = <img src={ richData?.icon } alt="" />;
} else if ( isEmptyURL ) {
icon = <Icon icon={ info } size={ 32 } />;
} else {
icon = <Icon icon={ globe } />;
}

return (
<div
aria-label={ __( 'Currently selected' ) }
Expand All @@ -47,6 +60,7 @@ export default function LinkPreview( {
'is-rich': hasRichData,
'is-fetching': !! isFetching,
'is-preview': true,
'is-error': isEmptyURL,
} ) }
>
<div className="block-editor-link-control__search-item-top">
Expand All @@ -59,22 +73,29 @@ export default function LinkPreview( {
}
) }
>
{ richData?.icon ? (
<img src={ richData?.icon } alt="" />
) : (
<Icon icon={ globe } />
) }
{ icon }
</span>
<span className="block-editor-link-control__search-item-details">
<ExternalLink
className="block-editor-link-control__search-item-title"
href={ value.url }
>
{ richData?.title || value?.title || displayURL }
</ExternalLink>
{ value?.url && (
<span className="block-editor-link-control__search-item-info">
{ displayURL }
{ ! isEmptyURL ? (
<>
<ExternalLink
className="block-editor-link-control__search-item-title"
href={ value.url }
>
{ richData?.title ||
value?.title ||
displayURL }
</ExternalLink>

{ value?.url && (
<span className="block-editor-link-control__search-item-info">
{ displayURL }
</span>
) }
</>
) : (
<span className="block-editor-link-control__search-item-error-notice">
Link is empty
</span>
) }
</span>
Expand Down
15 changes: 15 additions & 0 deletions packages/block-editor/src/components/link-control/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ $preview-image-height: 140px;
flex: 1; // fill available space.
}

&.is-error .block-editor-link-control__search-item-header {
align-items: center;
}

.block-editor-link-control__search-item-details {
overflow: hidden; // clip to force text ellipsis.
}
Expand All @@ -200,6 +204,12 @@ $preview-image-height: 140px;
}
}

&.is-error .block-editor-link-control__search-item-icon {
top: 0;
width: 32px;
max-height: 32px;
}


.block-editor-link-control__search-item-info,
.block-editor-link-control__search-item-title {
Expand Down Expand Up @@ -241,6 +251,11 @@ $preview-image-height: 140px;
line-height: 1.3;
}

.block-editor-link-control__search-item-error-notice {
font-style: italic;
font-size: 1.1em;
}

.block-editor-link-control__search-item-type {
display: block;
padding: 3px 8px;
Expand Down
34 changes: 34 additions & 0 deletions packages/block-editor/src/components/link-control/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,40 @@ describe( 'Basic rendering', () => {

expect( isEditing() ).toBe( false );
} );

it( 'should display human friendly error message if value URL prop is empty when component is forced into no-editing (preview) mode', async () => {
// Why do we need this test?
// Occasionally `forceIsEditingLink` is set explictly to `false` which causes the Link UI to render
// it's preview even if the `value` has no URL.
// for an example of this see the usage in the following file whereby forceIsEditingLink is used to start/stop editing mode:
// https://github.com/WordPress/gutenberg/blob/fa5728771df7cdc86369f7157d6aa763649937a7/packages/format-library/src/link/inline.js#L151.
// see also: https://github.com/WordPress/gutenberg/issues/17972.

const valueWithEmptyURL = {
url: '',
id: 123,
type: 'post',
};

act( () => {
render(
<LinkControl
value={ valueWithEmptyURL }
forceIsEditingLink={ false }
/>,
container
);
} );

const linkPreview = queryByRole( container, 'generic', {
name: 'Currently selected',
} );

const isPreviewError = linkPreview.classList.contains( 'is-error' );
expect( isPreviewError ).toBe( true );

expect( queryByText( linkPreview, 'Link is empty' ) ).toBeTruthy();
} );
} );

describe( 'Unlinking', () => {
Expand Down