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

Warn if a template does not have one main element #68656

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from
+67 −1
Draft

Warn if a template does not have one main element #68656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions packages/editor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,10 @@ Monitors local autosaves of a post in the editor. It uses several hooks and func

The module also checks for sessionStorage support and conditionally exports the `LocalAutosaveMonitor` component based on that.

### MainElementWarnings

Undocumented declaration.

### MediaPlaceholder

> **Deprecated** since 5.3, use `wp.blockEditor.MediaPlaceholder` instead.
Expand Down
6 changes: 5 additions & 1 deletion packages/editor/src/components/editor-interface/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import SavePublishPanels from '../save-publish-panels';
import TextEditor from '../text-editor';
import VisualEditor from '../visual-editor';
import EditorContentSlotFill from './content-slot-fill';
import MainElementWarnings from '../main-element-warnings';

const interfaceLabels = {
/* translators: accessibility text for the editor top bar landmark region. */
Expand Down Expand Up @@ -139,7 +140,10 @@ export default function EditorInterface( {
content={
<>
{ ! isDistractionFree && ! isPreviewMode && (
<EditorNotices />
<>
<EditorNotices />
<MainElementWarnings />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was not sure about this and would welcome suggestions for better places to trigger the check from.

</>
) }

<EditorContentSlotFill.Slot>
Expand Down
1 change: 1 addition & 0 deletions packages/editor/src/components/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export { default as EntitiesSavedStates } from './entities-saved-states';
export { useIsDirty as useEntitiesSavedStatesIsDirty } from './entities-saved-states/hooks/use-is-dirty';
export { default as ErrorBoundary } from './error-boundary';
export { default as LocalAutosaveMonitor } from './local-autosave-monitor';
export { default as MainElementWarnings } from './main-element-warnings';
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to make this component a public API? Probably better to have it for internal use only for now.

export { default as PageAttributesCheck } from './page-attributes/check';
export { default as PageAttributesOrder } from './page-attributes/order';
export { default as PageAttributesPanel } from './page-attributes/panel';
Expand Down
57 changes: 57 additions & 0 deletions packages/editor/src/components/main-element-warnings/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useSelect, useDispatch } from '@wordpress/data';
import { useEffect } from '@wordpress/element';
import { store as blockEditorStore } from '@wordpress/block-editor';
import { store as noticesStore } from '@wordpress/notices';

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

const checkMainTag = ( blocks, mainTagCount ) => {
blocks.forEach( ( block ) => {
if ( block.attributes.tagName === 'main' ) {
mainTagCount++;
}
// If the block has innerBlocks, call the function again.
if ( block.innerBlocks.length > 0 ) {
mainTagCount = checkMainTag( block.innerBlocks, mainTagCount );
}
} );

return mainTagCount;
};
Comment on lines +15 to +27
Copy link
Member

Choose a reason for hiding this comment

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

I like the direction here. I think we could optimize this by switching to the for...of loop and bailing out when count > 1.

If I remember correctly, Array.forEach doesn't allow the iteration execution to stop.

Here's what I've in mind:

// Returns 0, 1 or 2. 0 and 2 are in violation. 1 is correct.
function checkMainTagCount( blocks = [], count = 0 ) {
	for ( const block of blocks ) {
		if ( block.attributes.tagName === 'main' ) {
			count++;
		}

		// Look no further; we've found a violation.
		if ( count > 1 ) {
			break;
		}

		// Check nested blocks.
		if ( block.innerBlocks.length > 0 ) {
			count = checkMainTagCount( block.innerBlocks, count );
		}
	}

	return count;
}


export default function MainElementWarnings() {
const { type, blocks } = useSelect( ( select ) => {
const postType = select( editorStore ).getCurrentPostType();
const { getBlocks } = select( blockEditorStore );
return {
type: postType,
blocks: getBlocks(),
Copy link
Member

Choose a reason for hiding this comment

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

We could move the post type check here and only return the blocks value when editing a template. This will avoid unnecessary tracking for other post types.

};
}, [] );

const { createWarningNotice, removeNotice } = useDispatch( noticesStore );

useEffect( () => {
if ( 'wp_template' === type ) {
const mainTagCount = checkMainTag( blocks, 0 );

removeNotice( 'edit-site-main-notice' );

if ( 0 === mainTagCount || 1 < mainTagCount ) {
createWarningNotice(
__( 'Your template should have exactly one main element.' ),
{ id: 'edit-site-main-notice' }
);
}
}
}, [ type, blocks, createWarningNotice, removeNotice ] );

return null;
}
Loading