Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
* WordPress dependencies
*/
import { useMemo, useState, useCallback } from '@wordpress/element';
import { useEntityRecords } from '@wordpress/core-data';
import { useDispatch } from '@wordpress/data';
import { useEntityRecords, store as coreStore } from '@wordpress/core-data';
import { useDispatch, useSelect } from '@wordpress/data';
import {
SelectControl,
Button,
Expand Down Expand Up @@ -47,6 +47,11 @@ export default function OverlayTemplatePartSelector( {

const { createErrorNotice } = useDispatch( noticesStore );

const currentTheme = useSelect(
( select ) => select( coreStore ).getCurrentTheme()?.stylesheet,
[]
);

// Track if we're currently creating a new overlay
const [ isCreating, setIsCreating ] = useState( false );

Expand Down Expand Up @@ -80,17 +85,13 @@ export default function OverlayTemplatePartSelector( {

const templatePartOptions = overlayTemplateParts.map(
( templatePart ) => {
const templatePartId = createTemplatePartId(
templatePart.theme,
templatePart.slug
);
const label = templatePart.title?.rendered
? decodeEntities( templatePart.title.rendered )
: templatePart.slug;

return {
label,
value: templatePartId,
value: templatePart.slug,
};
}
);
Expand All @@ -103,13 +104,9 @@ export default function OverlayTemplatePartSelector( {
if ( ! overlay || ! overlayTemplateParts ) {
return null;
}
return overlayTemplateParts.find( ( templatePart ) => {
const templatePartId = createTemplatePartId(
templatePart.theme,
templatePart.slug
);
return templatePartId === overlay;
} );
return overlayTemplateParts.find(
( templatePart ) => templatePart.slug === overlay
);
}, [ overlay, overlayTemplateParts ] );

const handleSelectChange = ( value ) => {
Expand All @@ -119,12 +116,21 @@ export default function OverlayTemplatePartSelector( {
};

const handleEditClick = () => {
if ( ! overlay || ! onNavigateToEntityRecord ) {
if (
! overlay ||
! selectedTemplatePart ||
! onNavigateToEntityRecord
) {
return;
}

// Resolve the full template part ID using theme
// Default to current theme if not set
const theme = selectedTemplatePart.theme || currentTheme;
const templatePartId = createTemplatePartId( theme, overlay );

onNavigateToEntityRecord( {
postId: overlay,
postId: templatePartId,
postType: 'wp_template_part',
} );
};
Expand All @@ -136,13 +142,19 @@ export default function OverlayTemplatePartSelector( {
const templatePart = await createOverlayTemplatePart();

setAttributes( {
overlay: templatePart.id,
overlay: templatePart.slug,
} );

// Navigate to the new overlay for editing
// Create the full ID using theme and slug
if ( onNavigateToEntityRecord ) {
const theme = templatePart.theme || currentTheme;
const templatePartId = createTemplatePartId(
theme,
templatePart.slug
);
onNavigateToEntityRecord( {
postId: templatePart.id,
postId: templatePartId,
postType: 'wp_template_part',
} );
}
Expand All @@ -168,6 +180,7 @@ export default function OverlayTemplatePartSelector( {
setAttributes,
onNavigateToEntityRecord,
createErrorNotice,
currentTheme,
] );

const isCreateButtonDisabled = isResolving || isCreating;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ import { close, Icon } from '@wordpress/icons';
import { Button } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { getColorClassName } from '@wordpress/block-editor';
import { useSelect } from '@wordpress/data';
import { store as coreStore } from '@wordpress/core-data';

/**
* Internal dependencies
*/
import OverlayMenuIcon from './overlay-menu-icon';
import { createTemplatePartId } from '../../template-part/edit/utils/create-template-part-id';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given that we are depending on external utils, we need to add some test coverage here.

I would add a quick unit tests that validates the behaviour the Nav block expects for createTemplatePartId. That way if the original implementation changes at all then these tests will catch it. We'll need to name the tests really well so that if they were to fail the PR author would understand that they can't simply modify the test because it is documenting behaviour expected by the Nav block.


export default function ResponsiveWrapper( {
children,
Expand All @@ -30,6 +33,11 @@ export default function ResponsiveWrapper( {
overlay,
onNavigateToEntityRecord,
} ) {
const currentTheme = useSelect(
( select ) => select( coreStore ).getCurrentTheme()?.stylesheet,
[]
);

if ( ! isResponsive ) {
return children;
}
Expand Down Expand Up @@ -80,8 +88,13 @@ export default function ResponsiveWrapper( {
const handleToggleClick = () => {
// If an overlay template part is selected, navigate to it instead of toggling
if ( overlay && onNavigateToEntityRecord ) {
const templatePartId = createTemplatePartId(
currentTheme,
overlay
);

onNavigateToEntityRecord( {
postId: overlay,
postId: templatePartId,
postType: 'wp_template_part',
} );
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ jest.mock( '../use-create-overlay', () => ( {
default: jest.fn(),
} ) );

// Mock useDispatch specifically to avoid needing to set up full data store
// Mock useDispatch and useSelect specifically to avoid needing to set up full data store
jest.mock( '@wordpress/data', () => ( {
useDispatch: jest.fn(),
useSelect: jest.fn(),
createSelector: jest.fn( ( fn ) => fn ),
createRegistrySelector: jest.fn( ( fn ) => fn ),
createReduxStore: jest.fn( () => ( {} ) ),
Expand Down Expand Up @@ -92,6 +93,7 @@ const allTemplateParts = [
describe( 'OverlayTemplatePartSelector', () => {
const mockCreateOverlayTemplatePart = jest.fn();
const mockCreateErrorNotice = jest.fn();
const { useSelect } = require( '@wordpress/data' );

beforeEach( () => {
jest.clearAllMocks();
Expand All @@ -107,6 +109,9 @@ describe( 'OverlayTemplatePartSelector', () => {
useDispatch.mockReturnValue( {
createErrorNotice: mockCreateErrorNotice,
} );
// Mock useSelect to return current theme
// The component calls: select( coreStore ).getCurrentTheme()?.stylesheet
useSelect.mockReturnValue( 'twentytwentyfive' );
} );

describe( 'Loading state', () => {
Expand Down Expand Up @@ -195,7 +200,7 @@ describe( 'OverlayTemplatePartSelector', () => {
).toBeInTheDocument();
} );

it( 'should call set the overlay attribute when an overlay is selected', async () => {
it( 'should store slug only when an overlay is selected', async () => {
const user = userEvent.setup();

useEntityRecords.mockReturnValue( {
Expand All @@ -210,10 +215,10 @@ describe( 'OverlayTemplatePartSelector', () => {
name: 'Overlay template',
} );

await user.selectOptions( select, 'twentytwentyfive//my-overlay' );
await user.selectOptions( select, 'my-overlay' );

expect( mockSetAttributes ).toHaveBeenCalledWith( {
overlay: 'twentytwentyfive//my-overlay',
overlay: 'my-overlay',
} );
} );

Expand All @@ -229,7 +234,7 @@ describe( 'OverlayTemplatePartSelector', () => {
render(
<OverlayTemplatePartSelector
{ ...defaultProps }
overlay="twentytwentyfive//my-overlay"
overlay="my-overlay"
/>
);

Expand All @@ -244,7 +249,7 @@ describe( 'OverlayTemplatePartSelector', () => {
} );
} );

it( 'should display selected overlay', () => {
it( 'should display selected overlay by slug', () => {
useEntityRecords.mockReturnValue( {
records: [ templatePart1 ],
isResolving: false,
Expand All @@ -254,15 +259,15 @@ describe( 'OverlayTemplatePartSelector', () => {
render(
<OverlayTemplatePartSelector
{ ...defaultProps }
overlay="twentytwentyfive//my-overlay"
overlay="my-overlay"
/>
);

const select = screen.getByRole( 'combobox', {
name: 'Overlay template',
} );

expect( select ).toHaveValue( 'twentytwentyfive//my-overlay' );
expect( select ).toHaveValue( 'my-overlay' );
} );
} );

Expand Down Expand Up @@ -293,7 +298,7 @@ describe( 'OverlayTemplatePartSelector', () => {
render(
<OverlayTemplatePartSelector
{ ...defaultProps }
overlay="twentytwentyfive//my-overlay"
overlay="my-overlay"
/>
);

Expand Down Expand Up @@ -322,7 +327,7 @@ describe( 'OverlayTemplatePartSelector', () => {
render(
<OverlayTemplatePartSelector
{ ...defaultProps }
overlay="twentytwentyfive//my-overlay"
overlay="my-overlay"
/>
);

Expand All @@ -345,7 +350,7 @@ describe( 'OverlayTemplatePartSelector', () => {
render(
<OverlayTemplatePartSelector
{ ...defaultProps }
overlay="twentytwentyfive//my-overlay"
overlay="my-overlay"
onNavigateToEntityRecord={ undefined }
/>
);
Expand All @@ -359,7 +364,7 @@ describe( 'OverlayTemplatePartSelector', () => {
expect( editButton ).toHaveAttribute( 'aria-disabled', 'true' );
} );

it( 'should navigate to focused overlay editor when edit button is clicked', async () => {
it( 'should navigate to focused overlay editor with full ID when edit button is clicked', async () => {
const user = userEvent.setup();

useEntityRecords.mockReturnValue( {
Expand All @@ -371,7 +376,7 @@ describe( 'OverlayTemplatePartSelector', () => {
render(
<OverlayTemplatePartSelector
{ ...defaultProps }
overlay="twentytwentyfive//my-overlay"
overlay="my-overlay"
/>
);

Expand All @@ -382,6 +387,7 @@ describe( 'OverlayTemplatePartSelector', () => {

await user.click( editButton );

// Should construct full ID from theme and slug
expect( mockOnNavigateToEntityRecord ).toHaveBeenCalledWith( {
postId: 'twentytwentyfive//my-overlay',
postType: 'wp_template_part',
Expand All @@ -400,7 +406,7 @@ describe( 'OverlayTemplatePartSelector', () => {
render(
<OverlayTemplatePartSelector
{ ...defaultProps }
overlay="twentytwentyfive//my-overlay"
overlay="my-overlay"
onNavigateToEntityRecord={ undefined }
/>
);
Expand Down Expand Up @@ -461,7 +467,7 @@ describe( 'OverlayTemplatePartSelector', () => {
} );

describe( 'Create overlay', () => {
it( 'should call createOverlayTemplatePart when create button is clicked', async () => {
it( 'should store slug only and navigate with full ID when creating overlay', async () => {
const user = userEvent.setup();
const newOverlay = {
id: 'twentytwentyfive//overlay',
Expand Down Expand Up @@ -490,9 +496,11 @@ describe( 'OverlayTemplatePartSelector', () => {
await user.click( createButton );

expect( mockCreateOverlayTemplatePart ).toHaveBeenCalled();
// Should store slug only
expect( mockSetAttributes ).toHaveBeenCalledWith( {
overlay: 'twentytwentyfive//overlay',
overlay: 'overlay',
} );
// Should navigate with full ID constructed from theme and slug
expect( mockOnNavigateToEntityRecord ).toHaveBeenCalledWith( {
postId: 'twentytwentyfive//overlay',
postType: 'wp_template_part',
Expand Down
Loading
Loading