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.

Sign up for GitHub

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

Replace Starter Content modal with inserter panel #66836

Merged
merged 17 commits into from
Dec 20, 2024
Merged

Replace Starter Content modal with inserter panel #66836

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1991eef
Inserter: Add 'Starter Content' category to the inserter
ellatrix Nov 7, 2024
45d6b89
Add cat condition
ellatrix Nov 7, 2024
6ac1ca7
Replace Starter Content modal with inserter panel
ellatrix Nov 7, 2024
9787d50
remove option condition
ellatrix Nov 11, 2024
2f20ab1
remove duplicated category
draganescu Nov 26, 2024
07c5db9
Attempt to fix media panel test
mikachan Dec 17, 2024
881d7b1
Attempt to fix style variations test
mikachan Dec 17, 2024
c6c423e
Attempt to fix recursive pattern test
mikachan Dec 18, 2024
2f958e0
Attempt to fix template resolution test
mikachan Dec 18, 2024
4f9af6c
Attempt to fix new page test
mikachan Dec 18, 2024
52b7849
Tweak template resolution test
mikachan Dec 18, 2024
45ec9af
Remove categoryObject
mikachan Dec 18, 2024
bfae8e3
Fix aria-current on category tabs
mikachan Dec 18, 2024
2ca5234
Fix selected cat in patterns explorer
mikachan Dec 18, 2024
8e8210f
pass postId so that the inserter gets reopened when a new page is cre…
scruffian Dec 18, 2024
0c888af
Take control of zoom if a new page is created
mikachan Dec 19, 2024
36a302d
Add post Id back
scruffian Dec 20, 2024
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
5 changes: 2 additions & 3 deletions packages/block-editor/src/components/inserter/block-patterns-explorer/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ import { usePatternCategories } from '../block-patterns-tab/use-pattern-categori

function PatternsExplorer( { initialCategory, rootClientId } ) {
const [ searchValue, setSearchValue ] = useState( '' );
const [ selectedCategory, setSelectedCategory ] = useState(
initialCategory?.name
);
const [ selectedCategory, setSelectedCategory ] =
useState( initialCategory );

const patternCategories = usePatternCategories( rootClientId );

Expand Down
4 changes: 3 additions & 1 deletion packages/block-editor/src/components/inserter/block-patterns-tab/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ function BlockPatternsTab( {
) }
{ showPatternsExplorer && (
<PatternsExplorerModal
initialCategory={ selectedCategory || categories[ 0 ] }
initialCategory={
selectedCategory?.name || categories[ 0 ]?.name
}
patternCategories={ categories }
onModalClose={ () => setShowPatternsExplorer( false ) }
rootClientId={ rootClientId }
Expand Down
8 changes: 4 additions & 4 deletions ...ages/block-editor/src/components/inserter/block-patterns-tab/pattern-category-previews.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,19 @@ export function PatternCategoryPreviews( {
return false;
}

if ( category.name === allPatternsCategory.name ) {
if ( category.name === allPatternsCategory?.name ) {
return true;
}

if (
category.name === myPatternsCategory.name &&
category.name === myPatternsCategory?.name &&
pattern.type === INSERTER_PATTERN_TYPES.user
) {
return true;
}

if (
category.name === starterPatternsCategory.name &&
category.name === starterPatternsCategory?.name &&
pattern.blockTypes?.includes( 'core/post-content' )
) {
return true;
Expand Down Expand Up @@ -149,7 +149,7 @@ export function PatternCategoryPreviews( {
level={ 4 }
as="div"
>
{ category.label }
{ category?.label }
</Heading>
</FlexBlock>
<PatternsFilter
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/components/inserter/block-patterns-tab/patterns-filter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
const getShouldDisableSyncFilter = ( sourceFilter ) =>
sourceFilter !== 'all' && sourceFilter !== 'user';
const getShouldHideSourcesFilter = ( category ) => {
return category.name === myPatternsCategory.name;
return category?.name === myPatternsCategory.name;
};

const PATTERN_SOURCE_MENU_OPTIONS = [
Expand Down Expand Up @@ -60,7 +60,7 @@ export function PatternsFilter( {
// the user may be confused when switching to another category if the haven't explicity set
// this filter themselves.
const currentPatternSourceFilter =
category.name === myPatternsCategory.name
category?.name === myPatternsCategory.name
? INSERTER_PATTERN_TYPES.user
: patternSourceFilter;

Expand Down
4 changes: 3 additions & 1 deletion packages/block-editor/src/components/inserter/category-tabs/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ function CategoryTabs( {
tabId={ category.name }
aria-label={ category.label }
aria-current={
category === selectedCategory ? 'true' : undefined
category.name === selectedCategory?.name
? 'true'
: undefined
}
>
{ category.label }
Expand Down
21 changes: 8 additions & 13 deletions packages/editor/src/components/preferences-modal/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import PageAttributesCheck from '../page-attributes/check';
import PostTypeSupportCheck from '../post-type-support-check';
import { store as editorStore } from '../../store';
import { unlock } from '../../lock-unlock';
import { useStartPatterns } from '../start-page-options';

const {
PreferencesModal,
Expand Down Expand Up @@ -73,7 +72,6 @@ function PreferencesModalContents( { extraSections = {} } ) {
const { setIsListViewOpened, setIsInserterOpened } =
useDispatch( editorStore );
const { set: setPreference } = useDispatch( preferencesStore );
const hasStarterPatterns = !! useStartPatterns().length;

const sections = useMemo(
() =>
Expand Down Expand Up @@ -114,16 +112,14 @@ function PreferencesModalContents( { extraSections = {} } ) {
'Allow right-click contextual menus'
) }
/>
{ hasStarterPatterns && (
<PreferenceToggleControl
scope="core"
featureName="enableChoosePatternModal"
help={ __(
'Shows starter patterns when creating a new page.'
) }
label={ __( 'Show starter patterns' ) }
/>
) }
<PreferenceToggleControl
scope="core"
featureName="enableChoosePatternModal"
help={ __(
'Shows starter patterns when creating a new page.'
) }
label={ __( 'Show starter patterns' ) }
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we remove the condition?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe because we want this setting to persist across themes, even if they don't have starter patterns?

</PreferencesModalSection>
<PreferencesModalSection
title={ __( 'Document settings' ) }
Expand Down Expand Up @@ -341,7 +337,6 @@ function PreferencesModalContents( { extraSections = {} } ) {
setIsListViewOpened,
setPreference,
isLargeViewport,
hasStarterPatterns,
]
);

Expand Down
125 changes: 14 additions & 111 deletions packages/editor/src/components/start-page-options/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,121 +1,18 @@
/**
* WordPress dependencies
*/
import { Modal } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useState, useMemo } from '@wordpress/element';
import {
store as blockEditorStore,
__experimentalBlockPatternsList as BlockPatternsList,
} from '@wordpress/block-editor';
import { useEffect } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';
import { store as coreStore } from '@wordpress/core-data';
import { __unstableSerializeAndClean } from '@wordpress/blocks';
import { store as preferencesStore } from '@wordpress/preferences';
import { store as interfaceStore } from '@wordpress/interface';

/**
* Internal dependencies
*/
import { store as editorStore } from '../../store';
import { TEMPLATE_POST_TYPE } from '../../store/constants';

export function useStartPatterns() {
// A pattern is a start pattern if it includes 'core/post-content' in its blockTypes,
// and it has no postTypes declared and the current post type is page or if
// the current post type is part of the postTypes declared.
const { blockPatternsWithPostContentBlockType, postType } = useSelect(
( select ) => {
const { getPatternsByBlockTypes, getBlocksByName } =
select( blockEditorStore );
const { getCurrentPostType, getRenderingMode } =
select( editorStore );
const rootClientId =
getRenderingMode() === 'post-only'
? ''
: getBlocksByName( 'core/post-content' )?.[ 0 ];
return {
blockPatternsWithPostContentBlockType: getPatternsByBlockTypes(
'core/post-content',
rootClientId
),
postType: getCurrentPostType(),
};
},
[]
);

return useMemo( () => {
if ( ! blockPatternsWithPostContentBlockType?.length ) {
return [];
}

/*
* Filter patterns without postTypes declared if the current postType is page
* or patterns that declare the current postType in its post type array.
*/
return blockPatternsWithPostContentBlockType.filter( ( pattern ) => {
return (
( postType === 'page' && ! pattern.postTypes ) ||
( Array.isArray( pattern.postTypes ) &&
pattern.postTypes.includes( postType ) )
);
} );
}, [ postType, blockPatternsWithPostContentBlockType ] );
}

function PatternSelection( { blockPatterns, onChoosePattern } ) {
const { editEntityRecord } = useDispatch( coreStore );
const { postType, postId } = useSelect( ( select ) => {
const { getCurrentPostType, getCurrentPostId } = select( editorStore );

return {
postType: getCurrentPostType(),
postId: getCurrentPostId(),
};
}, [] );
return (
<BlockPatternsList
blockPatterns={ blockPatterns }
onClickPattern={ ( _pattern, blocks ) => {
editEntityRecord( 'postType', postType, postId, {
blocks,
content: ( { blocks: blocksForSerialization = [] } ) =>
__unstableSerializeAndClean( blocksForSerialization ),
} );
onChoosePattern();
} }
/>
);
}

function StartPageOptionsModal( { onClose } ) {
const startPatterns = useStartPatterns();
const hasStartPattern = startPatterns.length > 0;

if ( ! hasStartPattern ) {
return null;
}

return (
<Modal
title={ __( 'Choose a pattern' ) }
isFullScreen
onRequestClose={ onClose }
>
<div className="editor-start-page-options__modal-content">
<PatternSelection
blockPatterns={ startPatterns }
onChoosePattern={ onClose }
/>
</div>
</Modal>
);
}

export default function StartPageOptions() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just remove this component and transform it into a hook?

const [ isClosed, setIsClosed ] = useState( false );
const shouldEnableModal = useSelect( ( select ) => {
const shouldEnable = useSelect( ( select ) => {
const { isEditedPostDirty, isEditedPostEmpty, getCurrentPostType } =
select( editorStore );
const preferencesModalActive =
Expand All @@ -129,13 +26,19 @@ export default function StartPageOptions() {
! preferencesModalActive &&
! isEditedPostDirty() &&
isEditedPostEmpty() &&
TEMPLATE_POST_TYPE !== getCurrentPostType()
'page' === getCurrentPostType()
);
}, [] );
const { setIsInserterOpened } = useDispatch( editorStore );

useEffect( () => {
if ( shouldEnable ) {
setIsInserterOpened( {
tab: 'patterns',
category: 'core/starter-content',
} );
}
}, [ shouldEnable, setIsInserterOpened ] );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the way this is built, it won't reopen the panel if you use the command palette to "create a new page" when you're already creating a new page. I think we should probably add the postId as a dependency.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my testing this is not the case - when you create a new page using the command palette, zoom out is invoked again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually on further testing, this is an issue.

Copy link
Member

Choose a reason for hiding this comment

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

After chatting with @scruffian, I think I have a work around for this in 0c888af.

This change should make sure we take control of the zoom level whenever a new page is created. I tested this by creating a new page, zooming in manually, then creating a new page via the command palette, and it should zoom out again automatically. Is this the behaviour we want?

Copy link
Contributor

Choose a reason for hiding this comment

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

looks good to me!

Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think you should remove this change though, I think we need both.


if ( ! shouldEnableModal || isClosed ) {
return null;
}

return <StartPageOptionsModal onClose={ () => setIsClosed( true ) } />;
return null;
}
3 changes: 0 additions & 3 deletions test/e2e/specs/editor/blocks/image.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,6 @@ test.describe( 'Image', () => {
page,
editor,
} ) => {
// This is a temp workaround for dragging and dropping images from the inserter.
// This should be removed when we have the zoom out view for media categories.
await page.setViewportSize( { width: 1400, height: 800 } );
Comment on lines -427 to -429
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this isn't needed anymore so I went ahead and removed it.

await editor.insertBlock( { name: 'core/image' } );
const imageBlock = editor.canvas.getByRole( 'document', {
name: 'Block: Image',
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/specs/editor/various/template-resolution.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,15 @@ test.describe( 'Template resolution', () => {
status: 'publish',
} );
await admin.editPost( newPage.id );
await page.locator( 'role=button[name="Block Inserter"i]' ).click();
await editor.openDocumentSettingsSidebar();
await expect(
page.getByRole( 'button', { name: 'Template options' } )
).toHaveText( 'Single Entries' );
await updateSiteSettings( { requestUtils, pageId: newPage.id } );
await page.reload();
await page.locator( 'role=button[name="Block Inserter"i]' ).click();
await editor.openDocumentSettingsSidebar();
await expect(
page.getByRole( 'button', { name: 'Template options' } )
).toHaveText( 'Index' );
Expand All @@ -81,6 +84,7 @@ test.describe( 'Template resolution', () => {
postType: 'page',
canvas: 'edit',
} );
await page.locator( 'role=button[name="Block Inserter"i]' ).click();
await editor.openDocumentSettingsSidebar();
await expect(
page.getByRole( 'button', { name: 'Template options' } )
Expand Down
4 changes: 1 addition & 3 deletions test/e2e/specs/site-editor/block-style-variations.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,7 @@ async function draftNewPage( page ) {

// Create a Group block with 2 nested Group blocks.
async function addPageContent( editor, page ) {
const inserterButton = page.locator(
'role=button[name="Block Inserter"i]'
);
const inserterButton = page.locator( 'role=tab[name="Blocks"i]' );
await inserterButton.click();
await page.type( 'role=searchbox[name="Search"i]', 'Group' );
await page.click(
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/specs/site-editor/pages.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ test.describe( 'Pages', () => {

// Create new page that has the default template so as to swap it.
await draftNewPage( page );
await page.locator( 'role=button[name="Block Inserter"i]' ).click();
await editor.openDocumentSettingsSidebar();
const templateOptionsButton = page
.getByRole( 'region', { name: 'Editor settings' } )
Expand All @@ -294,6 +295,7 @@ test.describe( 'Pages', () => {
} );

// Now reset, and apply the default template back.
await editor.openDocumentSettingsSidebar();
await templateOptionsButton.click();
const resetButton = page
.getByRole( 'menu', { name: 'Template options' } )
Expand All @@ -308,6 +310,7 @@ test.describe( 'Pages', () => {
editor,
} ) => {
await draftNewPage( page );
await page.locator( 'role=button[name="Block Inserter"i]' ).click();
await editor.openDocumentSettingsSidebar();
const templateOptionsButton = page
.getByRole( 'region', { name: 'Editor settings' } )
Expand Down
Loading