Skip to content

Commit

Permalink
Only show rich preview for image and description if data is available (
Browse files Browse the repository at this point in the history
…#33660)

* Only render lower UI if one or more pieces of relevant data are available

If we don't do this then we might see the placeholders for desc and image just because we have a icon.

* Add test coverage

* Ensure loading placeholders are visible when fetching

* Update test coverage

* Fix bug
  • Loading branch information
getdave authored Jul 29, 2021
1 parent 2a319f6 commit 055af1f
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 33 deletions.
65 changes: 36 additions & 29 deletions packages/block-editor/src/components/link-control/link-preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,36 +90,43 @@ export default function LinkPreview( {
<ViewerSlot fillProps={ value } />
</div>

{ ( hasRichData || isFetching ) && (
{ ( ( hasRichData &&
( richData?.image || richData?.description ) ) ||
isFetching ) && (
<div className="block-editor-link-control__search-item-bottom">
<div
aria-hidden={ ! richData?.image }
className={ classnames(
'block-editor-link-control__search-item-image',
{
'is-placeholder': ! richData?.image,
}
) }
>
{ richData?.image && (
<img src={ richData?.image } alt="" />
) }
</div>
<div
aria-hidden={ ! richData?.description }
className={ classnames(
'block-editor-link-control__search-item-description',
{
'is-placeholder': ! richData?.description,
}
) }
>
{ richData?.description && (
<Text truncate numberOfLines="2">
{ richData.description }
</Text>
) }
</div>
{ ( richData?.image || isFetching ) && (
<div
aria-hidden={ ! richData?.image }
className={ classnames(
'block-editor-link-control__search-item-image',
{
'is-placeholder': ! richData?.image,
}
) }
>
{ richData?.image && (
<img src={ richData?.image } alt="" />
) }
</div>
) }

{ ( richData?.description || isFetching ) && (
<div
aria-hidden={ ! richData?.description }
className={ classnames(
'block-editor-link-control__search-item-description',
{
'is-placeholder': ! richData?.description,
}
) }
>
{ richData?.description && (
<Text truncate numberOfLines="2">
{ richData.description }
</Text>
) }
</div>
) }
</div>
) }
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,6 @@ $preview-image-height: 140px;

.block-editor-link-control__search-item-bottom {
transition: opacity 1.5s;
min-height: 100px;
width: 100%;
}

Expand Down
46 changes: 43 additions & 3 deletions packages/block-editor/src/components/link-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1878,6 +1878,46 @@ describe( 'Rich link previews', () => {
expect( isRichLinkPreview ).toBe( true );
} );

it( 'should not display placeholders for the image and description if neither is available in the data', async () => {
mockFetchRichUrlData.mockImplementation( () =>
Promise.resolve( {
title: '',
icon: 'https://s.w.org/favicon.ico?2',
description: '',
image: '',
} )
);

act( () => {
render(
<LinkControl value={ selectedLink } hasRichPreviews />,
container
);
} );

// mockFetchRichUrlData resolves on next "tick" of event loop
await act( async () => {
await eventLoopTick();
} );

const linkPreview = container.querySelector(
"[aria-label='Currently selected']"
);

// Todo: refactor to use user-facing queries.
const hasRichImagePreview = linkPreview.querySelector(
'.block-editor-link-control__search-item-image'
);

// Todo: refactor to use user-facing queries.
const hasRichDescriptionPreview = linkPreview.querySelector(
'.block-editor-link-control__search-item-description'
);

expect( hasRichImagePreview ).toBeFalsy();
expect( hasRichDescriptionPreview ).toBeFalsy();
} );

it( 'should display a fallback when title is missing from rich data', async () => {
mockFetchRichUrlData.mockImplementation( () =>
Promise.resolve( {
Expand Down Expand Up @@ -1958,7 +1998,7 @@ describe( 'Rich link previews', () => {
} );

it.each( [ 'image', 'description' ] )(
'should display a fallback placeholder when %s it is missing from the rich data',
'should not display the rich %s when it is missing from the data',
async ( dataItem ) => {
mockFetchRichUrlData.mockImplementation( () => {
const data = {
Expand Down Expand Up @@ -1995,10 +2035,10 @@ describe( 'Rich link previews', () => {
expect( isRichLinkPreview ).toBe( true );

const missingDataItem = linkPreview.querySelector(
`.block-editor-link-control__search-item-${ dataItem }.is-placeholder`
`.block-editor-link-control__search-item-${ dataItem }`
);

expect( missingDataItem ).toBeTruthy();
expect( missingDataItem ).toBeFalsy();
}
);

Expand Down

0 comments on commit 055af1f

Please sign in to comment.