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.

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

Alternative fix: set persisted menu id when no menus or missing menu #32313

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 10 additions & 0 deletions packages/e2e-tests/specs/experiments/navigation-editor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ const REST_SEARCH_ROUTES = [
`rest_route=${ encodeURIComponent( '/wp/v2/search' ) }`,
];

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).
Expand Down Expand Up @@ -135,6 +140,10 @@ function getSearchMocks( responsesByMethod ) {
return getEndpointMocks( REST_SEARCH_ROUTES, responsesByMethod );
}

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

async function visitNavigationEditor() {
const query = addQueryArgs( '', {
page: 'gutenberg-navigation',
Expand Down Expand Up @@ -184,6 +193,7 @@ describe( 'Navigation editor', () => {
POST: menuPostResponse,
} ),
...getMenuItemMocks( { GET: [] } ),
...getPagesMocks( { GET: [ {} ] } ), // mock a single page
] );

await page.keyboard.type( 'Main Menu' );
Expand Down
20 changes: 16 additions & 4 deletions packages/edit-navigation/src/hooks/use-menu-entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,28 @@ export default function useMenuEntity( menuId ) {
const { editEntityRecord } = useDispatch( coreStore );

const menuEntityData = [ MENU_KIND, MENU_POST_TYPE, menuId ];
const editedMenu = useSelect(
( select ) =>
menuId &&
select( coreStore ).getEditedEntityRecord( ...menuEntityData ),
const { editedMenu, hasLoadedEditedMenu } = useSelect(
( select ) => {
return {
editedMenu:
menuId &&
select( coreStore ).getEditedEntityRecord(
...menuEntityData
),
hasLoadedEditedMenu: select(
coreStore
).hasFinishedResolution( 'getEditedEntityRecord', [
...menuEntityData,
] ),
};
},
[ menuId ]
);

return {
editedMenu,
menuEntityData,
editMenuEntityRecord: editEntityRecord,
hasLoadedEditedMenu,
};
}
14 changes: 13 additions & 1 deletion packages/edit-navigation/src/hooks/use-navigation-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { store as coreStore } from '@wordpress/core-data';
*/
import { store as editNavigationStore } from '../store';
import { useSelectedMenuId } from './index';
import useMenuEntity from './use-menu-entity';

const getMenusData = ( select ) => {
const selectors = select( 'core' );
Expand All @@ -29,8 +30,19 @@ export default function useNavigationEditor() {
const [ hasFinishedInitialLoad, setHasFinishedInitialLoad ] = useState(
false
);
const { editedMenu, hasLoadedEditedMenu } = useMenuEntity( selectedMenuId );
const { menus, hasLoadedMenus } = useSelect( getMenusData, [] );

/**
* If the Menu being edited has been requested from API and it has
* no values then it has been deleted so reset the selected menu ID.
*/
useEffect( () => {
if ( hasLoadedEditedMenu && ! Object.keys( editedMenu )?.length ) {
setSelectedMenuId( null );
}
}, [ hasLoadedEditedMenu, editedMenu ] );

const { createErrorNotice, createInfoNotice } = useDispatch( noticesStore );
const isMenuBeingDeleted = useSelect(
( select ) =>
Expand Down Expand Up @@ -67,7 +79,7 @@ export default function useNavigationEditor() {
force: true,
} );
if ( didDeleteMenu ) {
setSelectedMenuId( 0 );
setSelectedMenuId( null );
createInfoNotice(
sprintf(
// translators: %s: the name of a menu.
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-navigation/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export function processingQueue( state, action ) {
*
* @return {Object} Updated state.
*/
export function selectedMenuId( state = 0, action ) {
export function selectedMenuId( state = null, action ) {
switch ( action.type ) {
case 'SET_SELECTED_MENU_ID':
return action.menuId;
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-navigation/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { buildNavigationPostId } from './utils';
* @return {number} The selected menu ID.
*/
export function getSelectedMenuId( state ) {
return state.selectedMenuId ?? 0;
return state.selectedMenuId ?? null;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-navigation/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ describe( 'processingQueue', () => {

describe( 'selectedMenuId', () => {
it( 'should apply default state', () => {
expect( selectedMenuId( undefined, {} ) ).toEqual( 0 );
expect( selectedMenuId( undefined, {} ) ).toEqual( null );
} );

it( 'should update when a new menu is selected', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-navigation/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe( 'getMenuItemForClientId', () => {
describe( 'getSelectedMenuId', () => {
it( 'returns default selected menu ID (zero)', () => {
const state = {};
expect( getSelectedMenuId( state ) ).toBe( 0 );
expect( getSelectedMenuId( state ) ).toBe( null );
} );

it( 'returns selected menu ID', () => {
Expand Down