Skip to content

Commit

Permalink
Flag invalid Navigation Link items. (WordPress#31716)
Browse files Browse the repository at this point in the history 
* mark items that no longer exist as invalid

* mock pages on mocked search response

Necessary as the invalid links logic calls `getEntityRecord`
on pages, and if they don't exist a 404 error is thrown, causing
tests using `mocklSearchResponse` to fail, as the pages being
looked up don't exist.

Using `createPages` does not fix this issue.

* remove redundant span from missing text

* make tooltip text readable
  • Loading branch information
@vcanales
vcanales authored Mar 14, 2022
1 parent 936ce3a commit fb3d84b
Show file tree
Hide file tree
Showing 3 changed files with 209 additions and 62 deletions.
206 changes: 145 additions & 61 deletions packages/block-library/src/navigation-link/edit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ToolbarButton,
Tooltip,
ToolbarGroup,
KeyboardShortcuts,
} from '@wordpress/components';
import { displayShortcut, isKeyboardEvent, ENTER } from '@wordpress/keycodes';
import { __, sprintf } from '@wordpress/i18n';
Expand Down Expand Up @@ -265,6 +266,64 @@ export const updateNavigationLinkBlockAttributes = (
} );
};

const useIsInvalidLink = ( kind, type, id ) => {
const isPostType =
kind === 'post-type' || type === 'post' || type === 'page';
const hasId = Number.isInteger( id );
const postStatus = useSelect(
( select ) => {
if ( ! isPostType ) {
return null;
}
const { getEntityRecord } = select( coreStore );
return getEntityRecord( 'postType', type, id )?.status;
},
[ isPostType, type, id ]
);

// Check Navigation Link validity if:
// 1. Link is 'post-type'.
// 2. It has an id.
// 3. It's neither null, nor undefined, as valid items might be either of those while loading.
// If those conditions are met, check if
// 1. The post status is published.
// 2. The Navigation Link item has no label.
// If either of those is true, invalidate.
const isInvalid =
isPostType && hasId && postStatus && 'trash' === postStatus;
const isDraft = 'draft' === postStatus;

return [ isInvalid, isDraft ];
};

const useMissingText = ( type ) => {
let missingText = '';

switch ( type ) {
case 'post':
/* translators: label for missing post in navigation link block */
missingText = __( 'Select post' );
break;
case 'page':
/* translators: label for missing page in navigation link block */
missingText = __( 'Select page' );
break;
case 'category':
/* translators: label for missing category in navigation link block */
missingText = __( 'Select category' );
break;
case 'tag':
/* translators: label for missing tag in navigation link block */
missingText = __( 'Select tag' );
break;
default:
/* translators: label for missing values in navigation link block */
missingText = __( 'Add link' );
}

return missingText;
};

/**
* Removes HTML from a given string.
* Note the does not provide XSS protection or otherwise attempt
Expand Down Expand Up @@ -329,6 +388,7 @@ export default function NavigationLinkEdit( {
clientId,
} ) {
const {
id,
label,
type,
opensInNewTab,
Expand All @@ -339,6 +399,8 @@ export default function NavigationLinkEdit( {
kind,
} = attributes;

const [ isInvalid, isDraft ] = useIsInvalidLink( kind, type, id );

const link = {
url,
opensInNewTab,
Expand Down Expand Up @@ -589,36 +651,23 @@ export default function NavigationLinkEdit( {
onKeyDown,
} );

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

const classes = classnames( 'wp-block-navigation-item__content', {
'wp-block-navigation-link__placeholder': ! url,
'wp-block-navigation-link__placeholder': ! url || isInvalid || isDraft,
} );

let missingText = '';
switch ( type ) {
case 'post':
/* translators: label for missing post in navigation link block */
missingText = __( 'Select post' );
break;
case 'page':
/* translators: label for missing page in navigation link block */
missingText = __( 'Select page' );
break;
case 'category':
/* translators: label for missing category in navigation link block */
missingText = __( 'Select category' );
break;
case 'tag':
/* translators: label for missing tag in navigation link block */
missingText = __( 'Select tag' );
break;
default:
/* translators: label for missing values in navigation link block */
missingText = __( 'Add link' );
}
const missingText = useMissingText( type, isInvalid, isDraft );
/* translators: Whether the navigation link is Invalid or a Draft. */
const placeholderText = `(${
isInvalid ? __( 'Invalid' ) : __( 'Draft' )
})`;
const tooltipText =
isInvalid || isDraft
? __( 'This item has been deleted, or is a draft' )
: __( 'This item is missing a link' );

return (
<Fragment>
Expand Down Expand Up @@ -677,46 +726,81 @@ export default function NavigationLinkEdit( {
{ /* eslint-enable */ }
{ ! url ? (
<div className="wp-block-navigation-link__placeholder-text">
<Tooltip
position="top center"
text={ __( 'This item is missing a link' ) }
>
<span>{ missingText }</span>
<Tooltip position="top center" text={ tooltipText }>
<>
<span>{ missingText }</span>
<span className="wp-block-navigation-link__missing_text-tooltip">
{ tooltipText }
</span>
</>
</Tooltip>
</div>
) : (
<RichText
ref={ ref }
identifier="label"
className="wp-block-navigation-item__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 }
withoutInteractiveFormatting
allowedFormats={ [
'core/bold',
'core/italic',
'core/image',
'core/strikethrough',
] }
onClick={ () => {
if ( ! url ) {
setIsLinkOpen( true );
}
} }
/>
<>
{ ! isInvalid && ! isDraft && (
<RichText
ref={ ref }
identifier="label"
className="wp-block-navigation-item__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 }
withoutInteractiveFormatting
allowedFormats={ [
'core/bold',
'core/italic',
'core/image',
'core/strikethrough',
] }
onClick={ () => {
if ( ! url ) {
setIsLinkOpen( true );
}
} }
/>
) }
{ ( isInvalid || isDraft ) && (
<div className="wp-block-navigation-link__placeholder-text wp-block-navigation-link__label">
<KeyboardShortcuts
shortcuts={ {
enter: () =>
isSelected &&
setIsLinkOpen( true ),
} }
/>
<Tooltip
position="top center"
text={ tooltipText }
>
<>
<span>
{
/* Trim to avoid trailing white space when the placeholder text is not present */
`${ label } ${ placeholderText }`.trim()
}
</span>
<span className="wp-block-navigation-link__missing_text-tooltip">
{ tooltipText }
</span>
</>
</Tooltip>
</div>
) }
</>
) }
{ isLinkOpen && (
<Popover
Expand Down
11 changes: 11 additions & 0 deletions packages/block-library/src/navigation-link/editor.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,18 @@
}
}

.wp-block-navigation-link__invalid-item {
color: #000;
}

.wp-block-navigation-link__missing_text-tooltip {
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
}
/**
* Menu item setup state. Is shown when a menu item has no URL configured.
*/
Expand Down
54 changes: 53 additions & 1 deletion packages/e2e-tests/specs/editor/blocks/navigation.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,47 @@ const POSTS_ENDPOINT = '/wp/v2/posts';
const PAGES_ENDPOINT = '/wp/v2/pages';
const DRAFT_PAGES_ENDPOINT = [ PAGES_ENDPOINT, { status: 'draft' } ];
const NAVIGATION_MENUS_ENDPOINT = '/wp/v2/navigation';
// todo: consolidate with logic found in navigation-editor tests
// https://github.com/WordPress/gutenberg/blob/trunk/packages/e2e-tests/specs/experiments/navigation-editor.test.js#L71
const REST_PAGES_ROUTES = [
'/wp/v2/pages',
`rest_route=${ encodeURIComponent( '/wp/v2/pages' ) }`,
];

/**
* Determines if a given URL matches any of a given collection of
* routes (expressed as substrings).
*
* @param {string} reqUrl the full URL to be tested for matches.
* @param {Array} routes array of strings to match against the URL.
*/
function matchUrlToRoute( reqUrl, routes ) {
return routes.some( ( route ) => reqUrl.includes( route ) );
}

function getEndpointMocks( matchingRoutes, responsesByMethod ) {
return [ 'GET', 'POST', 'DELETE', 'PUT' ].reduce( ( mocks, restMethod ) => {
if ( responsesByMethod[ restMethod ] ) {
return [
...mocks,
{
match: ( request ) =>
matchUrlToRoute( request.url(), matchingRoutes ) &&
request.method() === restMethod,
onRequestMatch: createJSONResponse(
responsesByMethod[ restMethod ]
),
},
];
}

return mocks;
}, [] );
}

function getPagesMocks( responsesByMethod ) {
return getEndpointMocks( REST_PAGES_ROUTES, responsesByMethod );
}

async function mockSearchResponse( items ) {
const mappedItems = items.map( ( { title, slug }, index ) => ( {
Expand All @@ -48,14 +89,25 @@ async function mockSearchResponse( items ) {
type: 'post',
url: `https://this/is/a/test/search/${ slug }`,
} ) );

await setUpResponseMocking( [
{
match: ( request ) =>
request.url().includes( `rest_route` ) &&
request.url().includes( `search` ),
onRequestMatch: createJSONResponse( mappedItems ),
},
...getPagesMocks( {
GET: [
{
type: 'page',
id: 1,
link: 'https://example.com/1',
title: {
rendered: 'My page',
},
},
],
} ),
] );
}

Expand Down

0 comments on commit fb3d84b

Please sign in to comment.