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

PostFormat suggestion: fix double parsing #56255

Closed
wants to merge 5 commits into from
Closed
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: 2 additions & 8 deletions docs/reference-guides/data/data-core-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,6 @@ _Returns_

Return the current block list.

_Parameters_

- _state_ `Object`:

_Returns_

- `Array`: Block list.
Expand Down Expand Up @@ -533,11 +529,9 @@ Returns state object prior to a specified optimist transaction ID, or `null` if

### getSuggestedPostFormat

Returns a suggested post format for the current post, inferred only if there is a single block within the post and it is of a type known to match a default post format. Returns null if the format cannot be determined.
> **Deprecated**

_Parameters_

- _state_ `Object`: Global application state.
Returns a suggested post format for the current post, inferred only if there is a single block within the post and it is of a type known to match a default post format. Returns null if the format cannot be determined.

_Returns_

Expand Down
27 changes: 13 additions & 14 deletions packages/editor/src/components/post-format/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import { Button, SelectControl } from '@wordpress/components';
import { useDispatch, useSelect } from '@wordpress/data';
import { useInstanceId } from '@wordpress/compose';
import { store as coreStore } from '@wordpress/core-data';
import { store as blockEditorStore } from '@wordpress/block-editor';

/**
* Internal dependencies
*/
import PostFormatCheck from './check';
import { store as editorStore } from '../../store';
import { getSuggestedPostFormat } from '../../utils/get-suggested-post-format';

// All WP post formats, sorted alphabetically by translated name.
export const POST_FORMATS = [
Expand Down Expand Up @@ -42,20 +44,17 @@ export default function PostFormat() {
const instanceId = useInstanceId( PostFormat );
const postFormatSelectorId = `post-format-selector-${ instanceId }`;

const { postFormat, suggestedFormat, supportedFormats } = useSelect(
( select ) => {
const { getEditedPostAttribute, getSuggestedPostFormat } =
select( editorStore );
const _postFormat = getEditedPostAttribute( 'format' );
const themeSupports = select( coreStore ).getThemeSupports();
return {
postFormat: _postFormat ?? 'standard',
suggestedFormat: getSuggestedPostFormat(),
supportedFormats: themeSupports.formats,
};
},
[]
);
const { postFormat, blocks, supportedFormats } = useSelect( ( select ) => {
const { getEditedPostAttribute } = select( editorStore );
const _postFormat = getEditedPostAttribute( 'format' );
const themeSupports = select( coreStore ).getThemeSupports();
return {
postFormat: _postFormat ?? 'standard',
blocks: select( blockEditorStore ).getBlocks(),
supportedFormats: themeSupports.formats,
};
}, [] );
const suggestedFormat = getSuggestedPostFormat( blocks );
Copy link
Member

Choose a reason for hiding this comment

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

It might be more performant to derive value inside the mapSelect, so we don't re-render the component on every block change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, will have to revert to using the selector

Copy link
Member

Choose a reason for hiding this comment

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

Here's my alternative PR #56679.


const formats = POST_FORMATS.filter( ( format ) => {
// Ensure current format is always in the set.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import { Button, PanelBody } from '@wordpress/components';
import { useDispatch, useSelect } from '@wordpress/data';
import { __, sprintf } from '@wordpress/i18n';
import { store as coreStore } from '@wordpress/core-data';
import { store as blockEditorStore } from '@wordpress/block-editor';

/**
* Internal dependencies
*/
import { POST_FORMATS } from '../post-format';
import { store as editorStore } from '../../store';
import { getSuggestedPostFormat } from '../../utils/get-suggested-post-format';

const getSuggestion = ( supportedFormats, suggestedPostFormat ) => {
const formats = POST_FORMATS.filter( ( format ) =>
Expand All @@ -33,19 +35,22 @@ const PostFormatSuggestion = ( {
);

export default function PostFormatPanel() {
const { currentPostFormat, suggestion } = useSelect( ( select ) => {
const { getEditedPostAttribute, getSuggestedPostFormat } =
select( editorStore );
const supportedFormats =
select( coreStore ).getThemeSupports().formats ?? [];
return {
currentPostFormat: getEditedPostAttribute( 'format' ),
suggestion: getSuggestion(
supportedFormats,
getSuggestedPostFormat()
),
};
}, [] );
const { currentPostFormat, supportedFormats, blocks } = useSelect(
( select ) => {
const { getEditedPostAttribute } = select( editorStore );
return {
currentPostFormat: getEditedPostAttribute( 'format' ),
supportedFormats:
select( coreStore ).getThemeSupports().formats,
blocks: select( blockEditorStore ).getBlocks(),
};
},
[]
);
const suggestion = getSuggestion(
supportedFormats ?? [],
getSuggestedPostFormat( blocks )
);
Comment on lines +50 to +53
Copy link
Member

Choose a reason for hiding this comment

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

Same here.


const { editPost } = useDispatch( editorStore );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ import { store as blockEditorStore } from '@wordpress/block-editor';
import { useState } from '@wordpress/element';
import { isBlobURL } from '@wordpress/blob';

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

function flattenBlocks( blocks ) {
const result = [];

Expand Down Expand Up @@ -66,14 +61,14 @@ function Image( block ) {

export default function PostFormatPanel() {
const [ isUploading, setIsUploading ] = useState( false );
const { editorBlocks, mediaUpload } = useSelect(
( select ) => ( {
editorBlocks: select( editorStore ).getEditorBlocks(),
mediaUpload: select( blockEditorStore ).getSettings().mediaUpload,
} ),
[]
);
const externalImages = flattenBlocks( editorBlocks ).filter(
const { blocks, mediaUpload } = useSelect( ( select ) => {
const { getBlocks, getSettings } = select( blockEditorStore );
return {
blocks: getBlocks(),
mediaUpload: getSettings().mediaUpload,
};
}, [] );
const externalImages = flattenBlocks( blocks ).filter(
( block ) =>
block.name === 'core/image' &&
block.attributes.url &&
Expand Down
12 changes: 8 additions & 4 deletions packages/editor/src/components/provider/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ class NativeEditorProvider extends Component {

componentDidUpdate( prevProps ) {
if ( ! prevProps.isReady && this.props.isReady ) {
const blocks = this.props.blocks;
const blocks =
this.props.getEditedPostAttribute( 'blocks' ) ||
parse( this.props.getEditedPostContent() );
Copy link
Member Author

Choose a reason for hiding this comment

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

There's something terribly wrong here on the native side, but that's out of the scope of this PR. The blocks in the block editor store should be available when the editor is ready, yet they aren't (causing some test failures). I've just restore the logic from getEditorBlocks, though obviously it's going to parse the content here.

const isUnsupportedBlock = ( { name } ) =>
name === getUnregisteredTypeHandlerName();
const unsupportedBlockNames = blocks
Expand All @@ -277,7 +279,10 @@ class NativeEditorProvider extends Component {
// Let's request the HTML from the component's state directly.
html = applyFilters( 'native.persist-html' );
} else {
html = serialize( this.props.blocks );
const blocks = this.props.getEditedPostAttribute( 'blocks' );
html = blocks?.length
? serialize( blocks )
: this.props.getEditedPostContent();
}

const hasChanges =
Expand Down Expand Up @@ -354,7 +359,6 @@ const ComposedNativeProvider = compose( [
withSelect( ( select ) => {
const {
__unstableIsEditorReady: isEditorReady,
getEditorBlocks,
getEditedPostAttribute,
getEditedPostContent,
getEditorSettings,
Expand All @@ -372,8 +376,8 @@ const ComposedNativeProvider = compose( [
return {
mode: getEditorMode(),
isReady: isEditorReady(),
blocks: getEditorBlocks(),
title: getEditedPostAttribute( 'title' ),
getEditedPostAttribute,
getEditedPostContent,
defaultEditorColors,
defaultEditorGradients,
Expand Down
62 changes: 4 additions & 58 deletions packages/editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
getFreeformContentHandlerName,
getDefaultBlockName,
__unstableSerializeAndClean,
parse,
} from '@wordpress/blocks';
import { isInTheFuture, getDate } from '@wordpress/date';
import { addQueryArgs, cleanForSlug } from '@wordpress/url';
Expand Down Expand Up @@ -819,53 +818,12 @@ export function getEditedPostPreviewLink( state ) {
* is a single block within the post and it is of a type known to match a
* default post format. Returns null if the format cannot be determined.
*
* @param {Object} state Global application state.
* @deprecated
*
* @return {?string} Suggested post format.
*/
export function getSuggestedPostFormat( state ) {
const blocks = getEditorBlocks( state );

if ( blocks.length > 2 ) return null;

let name;
// If there is only one block in the content of the post grab its name
// so we can derive a suitable post format from it.
if ( blocks.length === 1 ) {
name = blocks[ 0 ].name;
// Check for core/embed `video` and `audio` eligible suggestions.
if ( name === 'core/embed' ) {
const provider = blocks[ 0 ].attributes?.providerNameSlug;
if ( [ 'youtube', 'vimeo' ].includes( provider ) ) {
name = 'core/video';
} else if ( [ 'spotify', 'soundcloud' ].includes( provider ) ) {
name = 'core/audio';
}
}
}

// If there are two blocks in the content and the last one is a text blocks
// grab the name of the first one to also suggest a post format from it.
if ( blocks.length === 2 && blocks[ 1 ].name === 'core/paragraph' ) {
name = blocks[ 0 ].name;
}

// We only convert to default post formats in core.
switch ( name ) {
case 'core/image':
return 'image';
case 'core/quote':
case 'core/pullquote':
return 'quote';
case 'core/gallery':
return 'gallery';
case 'core/video':
return 'video';
case 'core/audio':
return 'audio';
default:
return null;
}
export function getSuggestedPostFormat() {
return null;
Copy link
Member

Choose a reason for hiding this comment

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

We should properly deprecate this selector.

}

/**
Expand Down Expand Up @@ -1098,21 +1056,9 @@ export const isPublishSidebarEnabled = createRegistrySelector(
/**
* Return the current block list.
*
* @param {Object} state
* @return {Array} Block list.
*/
export const getEditorBlocks = createSelector(
( state ) => {
return (
getEditedPostAttribute( state, 'blocks' ) ||
parse( getEditedPostContent( state ) )
);
},
( state ) => [
getEditedPostAttribute( state, 'blocks' ),
getEditedPostContent( state ),
]
);
export const getEditorBlocks = getBlockEditorSelector( 'getBlocks' );

/**
* A block selection object.
Expand Down
Loading
Loading