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

Parser: Refactor meta parsing as filtered external source #5077

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
11 changes: 10 additions & 1 deletion blocks/api/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { mapValues, omit } from 'lodash';
* WordPress dependencies
*/
import { autop } from '@wordpress/autop';
import { applyFilters } from '@wordpress/hooks';

/**
* Internal dependencies
Expand Down Expand Up @@ -118,7 +119,15 @@ export function getBlockAttribute( attributeKey, attributeSchema, innerHTML, com
break;
}

return value === undefined ? attributeSchema.default : asType( value, attributeSchema.type );
value = applyFilters( 'blocks.getBlockAttribute.source', value, attributeSchema, innerHTML, commentAttributes );
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need two filters? blocks.getBlockAttribute and blocks.getBlockAttribute.source


if ( value === undefined ) {
value = attributeSchema.default;
} else {
value = asType( value, attributeSchema.type );
}

return applyFilters( 'blocks.getBlockAttribute', value, attributeSchema, innerHTML, commentAttributes );
}

/**
Expand Down
4 changes: 4 additions & 0 deletions edit-post/hooks/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* Internal dependencies
*/
import './meta';
28 changes: 28 additions & 0 deletions edit-post/hooks/meta/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd expect this file to live in editor.hooks for now. I understand that it's post related but I believe the refactoring to extract all post related stuff should be done in one step instead of introducing inconsistency.

* WordPress dependencies
*/
import { addFilter } from '@wordpress/hooks';
import { select } from '@wordpress/data';

const editor = select( 'core/editor' );

/**
* Filters an attribute value during parse to inject post properties from meta.
*
* @param {*} value Parsed value.
* @param {Object} schema Attribute schema.
*
* @return {*} Filtered value with meta substitute if applicable.
*/
function getMetaAttributeFromPost( value, schema ) {
if ( schema.source === 'meta' ) {
const meta = editor.getCurrentPost().meta;
if ( meta && meta.hasOwnProperty( schema.meta ) ) {
return meta[ schema.meta ];
}
}

return value;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

So right now, retrieving the attribute is done using a filter but updating this source type is still achieved using hard-coded code. I believe we should create a filter for it aswell?


addFilter( 'blocks.getBlockAttribute', 'core/edit-post/meta/getMetaAttributeFromPost', getMetaAttributeFromPost );
1 change: 1 addition & 0 deletions edit-post/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { EditorProvider, ErrorBoundary } from '@wordpress/editor';
import './assets/stylesheets/main.scss';
import Layout from './components/layout';
import store from './store';
import './hooks';

// Configure moment globally
moment.locale( dateSettings.l10n.locale );
Expand Down
24 changes: 12 additions & 12 deletions editor/store/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,21 @@ export default {

dispatch( savePost() );
},
SETUP_EDITOR( action ) {
SETUP_EDITOR( action, store ) {
const { post, settings } = action;
const effects = [];

// Dispatch must occur immediately, as subsequent block parse filtering
// may rely on post properties.
store.dispatch( resetPost( post ) );

// Include auto draft title in edits while not flagging post as dirty
if ( post.status === 'auto-draft' ) {
effects.push( setupNewPost( {
title: post.title.raw,
} ) );
}

// Parse content as blocks
if ( post.content.raw ) {
effects.push( resetBlocks( parse( post.content.raw ) ) );
Expand All @@ -311,17 +322,6 @@ export default {
effects.push( resetBlocks( blocks ) );
}

// Resetting post should occur after blocks have been reset, since it's
// the post reset that restarts history (used in dirty detection).
effects.push( resetPost( post ) );

// Include auto draft title in edits while not flagging post as dirty
if ( post.status === 'auto-draft' ) {
effects.push( setupNewPost( {
title: post.title.raw,
} ) );
}

return effects;
},
FETCH_REUSABLE_BLOCKS( action, store ) {
Expand Down
2 changes: 2 additions & 0 deletions editor/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import applyMiddlewares from './middlewares';
import {
getBlockCount,
getBlocks,
getCurrentPost,
getEditedPostAttribute,
getLastMultiSelectedBlockUid,
getSelectedBlockCount,
Expand All @@ -30,6 +31,7 @@ loadAndPersist( store, reducer, 'preferences', STORAGE_KEY );
registerSelectors( MODULE_KEY, {
getBlockCount,
getBlocks,
getCurrentPost,
getEditedPostAttribute,
getLastMultiSelectedBlockUid,
getSelectedBlockCount,
Expand Down
10 changes: 5 additions & 5 deletions editor/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ function getFlattenedBlocks( blocks ) {
export const editor = flow( [
combineReducers,

// Track undo history, starting at editor initialization.
partialRight( withHistory, { resetTypes: [ 'SETUP_NEW_POST', 'SETUP_EDITOR' ] } ),
// Track undo history, starting at editor initialization of blocks. Assumes
// one of either block resetting, new post setup occurs in initialization.
partialRight( withHistory, { resetTypes: [ 'SETUP_NEW_POST', 'RESET_BLOCKS' ] } ),

// Track whether changes exist, resetting at each post save. Relies on
// editor initialization firing post reset as an effect.
partialRight( withChangeDetection, { resetTypes: [ 'SETUP_NEW_POST', 'RESET_POST' ] } ),
// Track whether changes exist, resetting at initialization and each save.
partialRight( withChangeDetection, { resetTypes: [ 'RESET_POST', 'SETUP_NEW_POST', 'RESET_BLOCKS' ] } ),
Copy link
Contributor

Choose a reason for hiding this comment

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

RESET_BLOCKS is also triggered in the text editor and it should not reset dirty detection in that case

Copy link
Member Author

Choose a reason for hiding this comment

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

RESET_BLOCKS is also triggered in the text editor and it should not reset dirty detection in that case

Sigh... maybe we need a dedicated "editor ready" action in addition to the SETUP_EDITOR one 😅

Copy link
Member

Choose a reason for hiding this comment

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

Should switching editors affect history? It's kind of a destructive action that, as a user, I wouldn't expect from just switching editor modes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should switching editors affect history? It's kind of a destructive action that, as a user, I wouldn't expect from just switching editor modes.

I'd look into preserving history across modes. I've experienced content loss firsthand through something like that (Calypso editor): some Heisenbug in Visual makes content disappear, toggle to Code, notice it's empty, come back to Visual, find no edit history to recover the content.

] )( {
edits( state = {}, action ) {
switch ( action.type ) {
Expand Down
32 changes: 0 additions & 32 deletions editor/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import {
map,
first,
get,
has,
last,
reduce,
compact,
find,
some,
Expand Down Expand Up @@ -422,47 +420,17 @@ export const getBlock = createSelector(
return null;
}

let { attributes } = block;

// Inject custom source attribute values.
//
// TODO: Create generic external sourcing pattern, not explicitly
// targeting meta attributes.
const type = getBlockType( block.name );
if ( type ) {
attributes = reduce( type.attributes, ( result, value, key ) => {
if ( value.source === 'meta' ) {
if ( result === attributes ) {
result = { ...result };
}

result[ key ] = getPostMeta( state, value.meta );
}

return result;
}, attributes );
}

return {
...block,
attributes,
innerBlocks: getBlocks( state, uid ),
};
},
( state, uid ) => [
get( state, [ 'editor', 'present', 'blocksByUid', uid ] ),
getBlockDependantsCacheBust( state, uid ),
get( state, [ 'editor', 'present', 'edits', 'meta' ] ),
get( state, 'currentPost.meta' ),
]
);

function getPostMeta( state, key ) {
return has( state, [ 'editor', 'present', 'edits', 'meta', key ] ) ?
get( state, [ 'editor', 'present', 'edits', 'meta', key ] ) :
get( state, [ 'currentPost', 'meta', key ] );
}

/**
* Returns all block objects for the current post being edited as an array in
* the order they appear in the post.
Expand Down
Loading