Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Fix Submenu using wrong binding
  • Loading branch information
getdave committed Oct 17, 2025
commit 3e398c282ef5e86c6ba3581aba40cea5dbb7f260
9 changes: 6 additions & 3 deletions packages/block-library/src/navigation-link/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,10 @@ export default function NavigationLinkEdit( {
anchor={ popoverAnchor }
onRemove={ removeLink }
onChange={ ( updatedValue ) => {
const { isEntityLink } = updateAttributes(
const {
isEntityLink,
attributes: freshAttributes,
} = updateAttributes(
updatedValue,
setAttributes,
attributes
Expand All @@ -553,9 +556,9 @@ export default function NavigationLinkEdit( {
// Only create bindings for entity links (posts, pages, taxonomies)
// Never create bindings for custom links (manual URLs)
if ( isEntityLink ) {
createBinding();
createBinding( freshAttributes );
} else {
clearBinding();
clearBinding( freshAttributes );
}
} }
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1153,6 +1153,7 @@ describe( 'updateAttributes', () => {

expect( result ).toEqual( {
isEntityLink: true,
attributes: expect.any( Object ),
} );
} );

Expand All @@ -1173,6 +1174,7 @@ describe( 'updateAttributes', () => {

expect( result ).toEqual( {
isEntityLink: false,
attributes: expect.any( Object ),
} );
} );

Expand All @@ -1190,6 +1192,7 @@ describe( 'updateAttributes', () => {

expect( result ).toEqual( {
isEntityLink: false,
attributes: expect.any( Object ),
} );
} );

Expand All @@ -1215,6 +1218,7 @@ describe( 'updateAttributes', () => {
// Should return false because the link was severed and converted to custom
expect( result ).toEqual( {
isEntityLink: false,
attributes: expect.any( Object ),
} );
} );

Expand All @@ -1240,6 +1244,7 @@ describe( 'updateAttributes', () => {
// Should return true because entity link is preserved
expect( result ).toEqual( {
isEntityLink: true,
attributes: expect.any( Object ),
} );
} );

Expand All @@ -1260,6 +1265,7 @@ describe( 'updateAttributes', () => {
// mailto links have kind: 'custom', so isEntityLink should be false
expect( result ).toEqual( {
isEntityLink: false,
attributes: expect.any( Object ),
} );
} );

Expand All @@ -1280,6 +1286,7 @@ describe( 'updateAttributes', () => {
// tel links have kind: 'custom', so isEntityLink should be false
expect( result ).toEqual( {
isEntityLink: false,
attributes: expect.any( Object ),
} );
} );

Expand All @@ -1300,6 +1307,7 @@ describe( 'updateAttributes', () => {

expect( result ).toEqual( {
isEntityLink: true,
attributes: expect.any( Object ),
} );
} );
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,6 @@ export const updateAttributes = (

return {
isEntityLink: !! finalId && finalKind !== 'custom',
attributes, // Return the computed attributes object
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,49 @@ export function useEntityBinding( { clientId, attributes } ) {
const hasCorrectBinding =
hasUrlBinding && metadata?.bindings?.url?.source === expectedSource;

const clearBinding = useCallback( () => {
// Only clear if there's actually a valid binding to clear
if ( hasCorrectBinding ) {
// Remove the URL binding by setting it to undefined
updateBlockBindings( { url: undefined } );
}
}, [ hasCorrectBinding, updateBlockBindings ] );

const createBinding = useCallback( () => {
const source =
kind === 'post-type' ? 'core/post-data' : 'core/term-data';
updateBlockBindings( {
url: {
source,
args: {
key: 'link',
const clearBinding = useCallback(
( freshAttributes ) => {
// Use fresh attributes if provided, otherwise fall back to closure attributes
Copy link
Contributor

Choose a reason for hiding this comment

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

TBH I haven't fully figured out why the closure attributes can be incorrect sometimes. I have a feeling that maybe it would be possible to get them correct right off the bat -- at least in the scenario I can think off, where the user selects e.g. a page link from the Link UI within a Nav Submenu, we have the information that the Nav Link is going to point to a page.

But the freshAttributes approach works and fixes the issue I observed, so let's go with it 👍

const kindToUse = freshAttributes?.kind ?? kind;
const expectedSourceForClear =
kindToUse === 'post-type' ? 'core/post-data' : 'core/term-data';
const hasCorrectBindingForClear =
hasUrlBinding &&
metadata?.bindings?.url?.source === expectedSourceForClear;

if ( hasCorrectBindingForClear ) {
updateBlockBindings( { url: undefined } );
}
},
[ hasUrlBinding, metadata, updateBlockBindings, kind ]
);

const createBinding = useCallback(
( freshAttributes ) => {
// Use fresh attributes if provided, otherwise fall back to closure attributes
const kindToUse = freshAttributes?.kind ?? kind;

// Avoid creating binding if no kind is provided
if ( ! kindToUse ) {
return;
}

// Default to post-type in case there is a need to support dynamic kinds
// in the future.
const source =
kindToUse === 'taxonomy' ? 'core/term-data' : 'core/post-data';

updateBlockBindings( {
url: {
source,
args: {
key: 'link',
},
},
},
} );
}, [ updateBlockBindings, kind ] );
} );
},
[ updateBlockBindings, kind ]
);

return {
hasUrlBinding: hasCorrectBinding,
Expand Down
9 changes: 6 additions & 3 deletions packages/block-library/src/navigation-submenu/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,10 @@ export default function NavigationSubmenuEdit( {
} }
onChange={ ( updatedValue ) => {
// updateAttributes determines the final state and returns metadata
const { isEntityLink } = updateAttributes(
const {
isEntityLink,
attributes: freshAttributes,
} = updateAttributes(
updatedValue,
setAttributes,
attributes
Expand All @@ -442,9 +445,9 @@ export default function NavigationSubmenuEdit( {
// Only create bindings for entity links (posts, pages, taxonomies)
// Never create bindings for custom links (manual URLs)
if ( isEntityLink ) {
createBinding();
createBinding( freshAttributes );
} else {
clearBinding();
clearBinding( freshAttributes );
}
} }
/>
Expand Down