Skip to content

Commit d7dd02f

Browse files
getdavescruffianellatrixockhamryanwelcher
committed
Fix Navigation Link broken state when bound entity is deleted (#73142)
Co-authored-by: getdave <[email protected]> Co-authored-by: scruffian <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: ockham <[email protected]> Co-authored-by: ryanwelcher <[email protected]> Co-authored-by: t-hamano <[email protected]>
1 parent 6a7bf43 commit d7dd02f

File tree

8 files changed

+406
-43
lines changed

8 files changed

+406
-43
lines changed

‎packages/block-library/src/navigation-link/edit.js‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -97,21 +97,35 @@ const useIsInvalidLink = ( kind, type, id, enabled ) => {
9797
const hasId = Number.isInteger( id );
9898
const blockEditingMode = useBlockEditingMode();
9999

100-
const postStatus = useSelect(
100+
const { postStatus, isDeleted } = useSelect(
101101
( select ) => {
102102
if ( ! isPostType ) {
103-
return null;
103+
return { postStatus: null, isDeleted: false };
104104
}
105105

106106
// Fetching the posts status is an "expensive" operation. Especially for sites with large navigations.
107107
// When the block is rendered in a template or other disabled contexts we can skip this check in order
108108
// to avoid all these additional requests that don't really add any value in that mode.
109109
if ( blockEditingMode === 'disabled' || ! enabled ) {
110-
return null;
110+
return { postStatus: null, isDeleted: false };
111111
}
112112

113-
const { getEntityRecord } = select( coreStore );
114-
return getEntityRecord( 'postType', type, id )?.status;
113+
const { getEntityRecord, hasFinishedResolution } =
114+
select( coreStore );
115+
const entityRecord = getEntityRecord( 'postType', type, id );
116+
const hasResolved = hasFinishedResolution( 'getEntityRecord', [
117+
'postType',
118+
type,
119+
id,
120+
] );
121+
122+
// If resolution has finished and entityRecord is undefined, the entity was deleted.
123+
const deleted = hasResolved && entityRecord === undefined;
124+
125+
return {
126+
postStatus: entityRecord?.status,
127+
isDeleted: deleted,
128+
};
115129
},
116130
[ isPostType, blockEditingMode, enabled, type, id ]
117131
);
@@ -121,11 +135,13 @@ const useIsInvalidLink = ( kind, type, id, enabled ) => {
121135
// 2. It has an id.
122136
// 3. It's neither null, nor undefined, as valid items might be either of those while loading.
123137
// If those conditions are met, check if
124-
// 1. The post status is published.
125-
// 2. The Navigation Link item has no label.
138+
// 1. The post status is trash (trashed).
139+
// 2. The entity doesn't exist (deleted).
126140
// If either of those is true, invalidate.
127141
const isInvalid =
128-
isPostType && hasId && postStatus && 'trash' === postStatus;
142+
isPostType &&
143+
hasId &&
144+
( isDeleted || ( postStatus && 'trash' === postStatus ) );
129145
const isDraft = 'draft' === postStatus;
130146

131147
return [ isInvalid, isDraft ];
@@ -248,7 +264,12 @@ export default function NavigationLinkEdit( {
248264
const { getBlocks } = useSelect( blockEditorStore );
249265

250266
// URL binding logic
251-
const { clearBinding, createBinding } = useEntityBinding( {
267+
const {
268+
clearBinding,
269+
createBinding,
270+
hasUrlBinding,
271+
isBoundEntityAvailable,
272+
} = useEntityBinding( {
252273
clientId,
253274
attributes,
254275
} );
@@ -405,14 +426,23 @@ export default function NavigationLinkEdit( {
405426
}
406427
);
407428

408-
if ( ! url || isInvalid || isDraft ) {
429+
if (
430+
! url ||
431+
isInvalid ||
432+
isDraft ||
433+
( hasUrlBinding && ! isBoundEntityAvailable )
434+
) {
409435
blockProps.onClick = () => {
410436
setIsLinkOpen( true );
411437
};
412438
}
413439

414440
const classes = clsx( 'wp-block-navigation-item__content', {
415-
'wp-block-navigation-link__placeholder': ! url || isInvalid || isDraft,
441+
'wp-block-navigation-link__placeholder':
442+
! url ||
443+
isInvalid ||
444+
isDraft ||
445+
( hasUrlBinding && ! isBoundEntityAvailable ),
416446
} );
417447

418448
const missingText = getMissingText( type );
@@ -531,9 +561,10 @@ export default function NavigationLinkEdit( {
531561
link={ attributes }
532562
onClose={ () => {
533563
setIsLinkOpen( false );
534-
// If there is no link then remove the auto-inserted block.
564+
// If there is no link and no binding, remove the auto-inserted block.
535565
// This avoids empty blocks which can provided a poor UX.
536-
if ( ! url ) {
566+
// Don't remove if binding exists (even if entity is unavailable) so user can fix it.
567+
if ( ! url && ! hasUrlBinding ) {
537568
onReplace( [] );
538569
} else if ( isNewLink.current ) {
539570
// If we just created a new link, select it

‎packages/block-library/src/navigation-link/editor.scss‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,10 @@
143143
text-transform: uppercase;
144144
}
145145
}
146+
147+
/**
148+
* Error text styling for missing entity help text.
149+
*/
150+
.navigation-link-control__error-text {
151+
color: $alert-red;
152+
}

‎packages/block-library/src/navigation-link/link-ui/index.js‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { useInstanceId } from '@wordpress/compose';
2626
*/
2727
import { LinkUIPageCreator } from './page-creator';
2828
import LinkUIBlockInserter from './block-inserter';
29+
import { useEntityBinding } from '../shared/use-entity-binding';
2930

3031
/**
3132
* Given the Link block's type attribute, return the query params to give to
@@ -66,7 +67,8 @@ export function getSuggestionsQuery( type, kind ) {
6667
}
6768

6869
function UnforwardedLinkUI( props, ref ) {
69-
const { label, url, opensInNewTab, type, kind, id, metadata } = props.link;
70+
const { label, url, opensInNewTab, type, kind, id } = props.link;
71+
const { clientId } = props;
7072
const postType = type || 'page';
7173

7274
const [ addingBlock, setAddingBlock ] = useState( false );
@@ -78,12 +80,11 @@ function UnforwardedLinkUI( props, ref ) {
7880
name: postType,
7981
} );
8082

81-
// Check if there's a URL binding with the new binding sources
82-
// Only enable handleEntities when there's actually a binding present
83-
const hasUrlBinding =
84-
( metadata?.bindings?.url?.source === 'core/post-data' ||
85-
metadata?.bindings?.url?.source === 'core/term-data' ) &&
86-
!! id;
83+
// Use the entity binding hook to get binding status
84+
const { isBoundEntityAvailable } = useEntityBinding( {
85+
clientId,
86+
attributes: props.link,
87+
} );
8788

8889
// Memoize link value to avoid overriding the LinkControl's internal state.
8990
// This is a temporary fix. See https://github.com/WordPress/gutenberg/issues/50976#issuecomment-1568226407.
@@ -152,7 +153,7 @@ function UnforwardedLinkUI( props, ref ) {
152153
onChange={ props.onChange }
153154
onRemove={ props.onRemove }
154155
onCancel={ props.onCancel }
155-
handleEntities={ hasUrlBinding }
156+
handleEntities={ isBoundEntityAvailable }
156157
renderControlBottom={ () => {
157158
// Don't show the tools when there is submitted link (preview state).
158159
if ( link?.url?.length ) {

‎packages/block-library/src/navigation-link/shared/controls.js‎

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,11 @@ export function Controls( { attributes, setAttributes, clientId } ) {
8787
}, [ url ] );
8888

8989
// Use the entity binding hook internally
90-
const { hasUrlBinding, clearBinding } = useEntityBinding( {
91-
clientId,
92-
attributes,
93-
} );
90+
const { hasUrlBinding, isBoundEntityAvailable, clearBinding } =
91+
useEntityBinding( {
92+
clientId,
93+
attributes,
94+
} );
9495

9596
// Get direct store dispatch to bypass setBoundAttributes wrapper
9697
const { updateBlockAttributes } = useDispatch( blockEditorStore );
@@ -111,8 +112,7 @@ export function Controls( { attributes, setAttributes, clientId } ) {
111112
};
112113

113114
useEffect( () => {
114-
// Checking for ! hasUrlBinding is a defensive check, as we would
115-
// only want to focus the input if the url is not bound to an entity.
115+
// Only want to focus the input if the url is not bound to an entity.
116116
if ( ! hasUrlBinding && shouldFocusURLInputRef.current ) {
117117
// focuses and highlights the url input value, giving the user
118118
// the ability to delete the value quickly or edit it.
@@ -165,12 +165,28 @@ export function Controls( { attributes, setAttributes, clientId } ) {
165165
__next40pxDefaultSize
166166
id={ inputId }
167167
label={ __( 'Link' ) }
168-
value={ inputValue ? safeDecodeURI( inputValue ) : '' }
168+
value={ ( () => {
169+
if ( hasUrlBinding && ! isBoundEntityAvailable ) {
170+
return '';
171+
}
172+
return inputValue ? safeDecodeURI( inputValue ) : '';
173+
} )() }
169174
autoComplete="off"
170175
type="url"
171176
disabled={ hasUrlBinding }
177+
aria-invalid={
178+
hasUrlBinding && ! isBoundEntityAvailable
179+
? 'true'
180+
: undefined
181+
}
182+
aria-describedby={ helpTextId }
183+
className={
184+
hasUrlBinding && ! isBoundEntityAvailable
185+
? 'navigation-link-control__input-with-error-suffix'
186+
: undefined
187+
}
172188
onChange={ ( newValue ) => {
173-
if ( hasUrlBinding ) {
189+
if ( isBoundEntityAvailable ) {
174190
return;
175191
}
176192

@@ -180,13 +196,13 @@ export function Controls( { attributes, setAttributes, clientId } ) {
180196
setInputValue( newValue );
181197
} }
182198
onFocus={ () => {
183-
if ( hasUrlBinding ) {
199+
if ( isBoundEntityAvailable ) {
184200
return;
185201
}
186202
lastURLRef.current = url;
187203
} }
188204
onBlur={ () => {
189-
if ( hasUrlBinding ) {
205+
if ( isBoundEntityAvailable ) {
190206
return;
191207
}
192208

@@ -204,11 +220,19 @@ export function Controls( { attributes, setAttributes, clientId } ) {
204220
} );
205221
} }
206222
help={
207-
hasUrlBinding && (
208-
<BindingHelpText
223+
hasUrlBinding && ! isBoundEntityAvailable ? (
224+
<MissingEntityHelpText
225+
id={ helpTextId }
209226
type={ attributes.type }
210227
kind={ attributes.kind }
211228
/>
229+
) : (
230+
isBoundEntityAvailable && (
231+
<BindingHelpText
232+
type={ attributes.type }
233+
kind={ attributes.kind }
234+
/>
235+
)
212236
)
213237
}
214238
suffix={
@@ -225,6 +249,11 @@ export function Controls( { attributes, setAttributes, clientId } ) {
225249
showTooltip
226250
label={ __( 'Unsync and edit' ) }
227251
__next40pxDefaultSize
252+
className={
253+
hasUrlBinding && ! isBoundEntityAvailable
254+
? 'navigation-link-control__error-suffix-button'
255+
: undefined
256+
}
228257
/>
229258
)
230259
}
@@ -306,3 +335,32 @@ function BindingHelpText( { type, kind } ) {
306335
entityType
307336
);
308337
}
338+
339+
/**
340+
* Component to display error help text for missing entity bindings.
341+
*
342+
* @param {Object} props - Component props
343+
* @param {string} props.id - ID for the help text element (for aria-describedby)
344+
* @param {string} props.type - The entity type
345+
* @param {string} props.kind - The entity kind
346+
* @return {JSX.Element} Error help text component
347+
*/
348+
function MissingEntityHelpText( { id, type, kind } ) {
349+
const entityType = getEntityTypeName( type, kind );
350+
return (
351+
<span
352+
id={ id }
353+
className="navigation-link-control__error-text"
354+
role="alert"
355+
aria-live="polite"
356+
>
357+
{ sprintf(
358+
/* translators: %s is the entity type (e.g., "page", "post", "category") */
359+
__(
360+
'Synced %s is missing. Please update or remove this link.'
361+
),
362+
entityType
363+
) }
364+
</span>
365+
);
366+
}

‎packages/block-library/src/navigation-link/shared/test/controls.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ jest.mock( '../../../utils/hooks', () => ( {
2828
jest.mock( '../use-entity-binding', () => ( {
2929
useEntityBinding: jest.fn( () => ( {
3030
hasUrlBinding: false,
31+
isBoundEntityAvailable: false,
3132
clearBinding: jest.fn(),
3233
} ) ),
3334
} ) );
@@ -202,6 +203,7 @@ describe( 'Controls', () => {
202203
const { useEntityBinding } = require( '../use-entity-binding' );
203204
useEntityBinding.mockReturnValue( {
204205
hasUrlBinding: true,
206+
isBoundEntityAvailable: true,
205207
clearBinding: jest.fn(),
206208
} );
207209

@@ -225,6 +227,7 @@ describe( 'Controls', () => {
225227
const { useEntityBinding } = require( '../use-entity-binding' );
226228
useEntityBinding.mockReturnValue( {
227229
hasUrlBinding: true,
230+
isBoundEntityAvailable: true,
228231
clearBinding: jest.fn(),
229232
} );
230233

@@ -262,6 +265,7 @@ describe( 'Controls', () => {
262265
const { useEntityBinding } = require( '../use-entity-binding' );
263266
useEntityBinding.mockReturnValue( {
264267
hasUrlBinding: true,
268+
isBoundEntityAvailable: true,
265269
clearBinding: jest.fn(),
266270
} );
267271

@@ -285,6 +289,7 @@ describe( 'Controls', () => {
285289
const { useEntityBinding } = require( '../use-entity-binding' );
286290
useEntityBinding.mockReturnValue( {
287291
hasUrlBinding: true,
292+
isBoundEntityAvailable: true,
288293
clearBinding: jest.fn(),
289294
} );
290295

‎packages/block-library/src/navigation-link/shared/test/use-entity-binding.js‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,23 @@ import {
1818
// Mock the entire @wordpress/block-editor module
1919
jest.mock( '@wordpress/block-editor', () => ( {
2020
useBlockBindingsUtils: jest.fn(),
21+
useBlockEditingMode: jest.fn(),
2122
} ) );
2223

24+
// Mock useSelect specifically to avoid needing to set up full data store
25+
jest.mock( '@wordpress/data/src/components/use-select', () => {
26+
const mock = jest.fn();
27+
return mock;
28+
} );
29+
2330
/**
2431
* WordPress dependencies
2532
*/
26-
import { useBlockBindingsUtils } from '@wordpress/block-editor';
33+
import {
34+
useBlockBindingsUtils,
35+
useBlockEditingMode,
36+
} from '@wordpress/block-editor';
37+
import { useSelect } from '@wordpress/data';
2738

2839
describe( 'useEntityBinding', () => {
2940
const mockUpdateBlockBindings = jest.fn();
@@ -33,6 +44,8 @@ describe( 'useEntityBinding', () => {
3344
useBlockBindingsUtils.mockReturnValue( {
3445
updateBlockBindings: mockUpdateBlockBindings,
3546
} );
47+
useBlockEditingMode.mockReturnValue( 'default' );
48+
useSelect.mockReturnValue( true );
3649
} );
3750

3851
describe( 'hasUrlBinding', () => {

0 commit comments

Comments
 (0)