Skip to content

Commit

Permalink
MetaBoxes: Remove dirty-checking metaboxes state
Browse files Browse the repository at this point in the history
  • Loading branch information
youknowriad committed Dec 27, 2017
1 parent 34c81b4 commit 9eff87c
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 279 deletions.
8 changes: 3 additions & 5 deletions docs/meta-box.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,9 @@ When rendering the Gutenberg Page, the metaboxes are rendered to a hidden div `#

#### MetaBoxArea Component

When the component renders it will store a ref to the metaboxes container, retrieve the metaboxes HTML from the prefetch location and watches input and changes.
When the component renders it will store a ref to the metaboxes container, retrieve the metaboxes HTML from the prefetch location.

The change detection will store the current form's `FormData`, then whenever a change is detected the current form data will be checked vs, the original form data. This serves as a way to see if the meta box state is dirty. When the meta box state has been detected to have changed, a Redux action `META_BOX_STATE_CHANGED` is dispatched, updating the store setting the isDirty flag to `true`. If the state ever returns back to the original form data, `META_BOX_STATE_CHANGED` is dispatched again to set the isDirty flag to `false`. A selector `isMetaBoxStateDirty()` is used to help check whether the post can be updated. It checks each meta box for whether it is dirty, and if there is at least one dirty meta box, it will return true. This dirty detection does not impact creating new posts, as the content will have to change before meta boxes can trigger the overall dirty state.

When the post is updated, only meta boxes areas that are active and dirty, will be submitted. This removes any unnecessary requests being made. No extra revisions, are created either by the meta box submissions. A Redux action will trigger on `REQUEST_POST_UPDATE` for any dirty meta box. See `editor/effects.js`. The `REQUEST_META_BOX_UPDATES` action will set that meta boxes' state to `isUpdating`, the `isUpdating` prop will be sent into the `MetaBoxArea` and cause a form submission.
When the post is updated, only meta boxes areas that are active will be submitted. This removes any unnecessary requests being made. No extra revisions, are created either by the meta box submissions. A Redux action will trigger on `REQUEST_POST_UPDATE` for any active meta box. See `editor/effects.js`. The `REQUEST_META_BOX_UPDATES` action will set that meta boxes' state to `isUpdating`, the `isUpdating` prop will be sent into the `MetaBoxArea` and cause a form submission.

If the metabox area is saving, we display an updating overlay, to prevent users from changing the form values while the meta box is submitting.

Expand All @@ -75,7 +73,7 @@ So an example url would look like:

This url is automatically passed into React via a `_wpMetaBoxUrl` global variable.

Thus page page mimics the `post.php` post form, so when it is submitted it will normally fire all of the necessary hooks and actions, and have the proper global state to correctly fire any PHP meta box mumbo jumbo without needing to modify any existing code. On successful submission, React will signal a `handleMetaBoxReload` to set up the new form state for dirty checking, remove the updating overlay, and set the store to no longer be updating the meta box area.
Thus page page mimics the `post.php` post form, so when it is submitted it will normally fire all of the necessary hooks and actions, and have the proper global state to correctly fire any PHP meta box mumbo jumbo without needing to modify any existing code. On successful submission, React will signal a `handleMetaBoxReload` to remove the updating overlay, and set the store to no longer be updating the meta box area.


### Common Compatibility Issues
Expand Down
33 changes: 2 additions & 31 deletions editor/components/meta-boxes/meta-boxes-area/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* External dependencies
*/
import { isEqual } from 'lodash';
import classnames from 'classnames';
import { connect } from 'react-redux';
import jQuery from 'jquery';
Expand All @@ -17,7 +16,7 @@ import { Spinner } from '@wordpress/components';
* Internal dependencies
*/
import './style.scss';
import { handleMetaBoxReload, metaBoxStateChanged, metaBoxLoaded } from '../../../store/actions';
import { handleMetaBoxReload, metaBoxLoaded } from '../../../store/actions';
import { getMetaBox, isSavingPost } from '../../../store/selectors';

class MetaBoxesArea extends Component {
Expand All @@ -29,7 +28,6 @@ class MetaBoxesArea extends Component {
};
this.originalFormData = '';
this.bindNode = this.bindNode.bind( this );
this.checkState = this.checkState.bind( this );
}

bindNode( node ) {
Expand All @@ -43,17 +41,9 @@ class MetaBoxesArea extends Component {

componentWillUnmount() {
this.mounted = false;
this.unbindFormEvents();
document.querySelector( '#metaboxes' ).appendChild( this.form );
}

unbindFormEvents() {
if ( this.form ) {
this.form.removeEventListener( 'change', this.checkState );
this.form.removeEventListener( 'input', this.checkState );
}
}

componentWillReceiveProps( nextProps ) {
if ( nextProps.isUpdating && ! this.props.isUpdating ) {
this.setState( { loading: true } );
Expand Down Expand Up @@ -84,30 +74,13 @@ class MetaBoxesArea extends Component {
this.node.appendChild( this.form );
this.form.onSubmit = ( event ) => event.preventDefault();
this.originalFormData = this.getFormData();
this.form.addEventListener( 'change', this.checkState );
this.form.addEventListener( 'input', this.checkState );
this.props.metaBoxLoaded( location );
}

getFormData() {
return jQuery( this.form ).serialize();
}

checkState() {
const { loading } = this.state;
const { isDirty, changedMetaBoxState, location } = this.props;

const newIsDirty = ! isEqual( this.originalFormData, this.getFormData() );

/**
* If we are not updating, then if dirty and equal to original, then set not dirty.
* If we are not updating, then if not dirty and not equal to original, set as dirty.
*/
if ( ! loading && isDirty !== newIsDirty ) {
changedMetaBoxState( location, newIsDirty );
}
}

render() {
const { location } = this.props;
const { loading } = this.state;
Expand All @@ -132,10 +105,9 @@ class MetaBoxesArea extends Component {

function mapStateToProps( state, ownProps ) {
const metaBox = getMetaBox( state, ownProps.location );
const { isDirty, isUpdating } = metaBox;
const { isUpdating } = metaBox;

return {
isDirty,
isUpdating,
isPostSaving: isSavingPost( state ) ? true : false,
};
Expand All @@ -145,7 +117,6 @@ function mapDispatchToProps( dispatch ) {
return {
// Used to set the reference to the MetaBox in redux, fired when the component mounts.
metaBoxReloaded: ( location ) => dispatch( handleMetaBoxReload( location ) ),
changedMetaBoxState: ( location, hasChanged ) => dispatch( metaBoxStateChanged( location, hasChanged ) ),
metaBoxLoaded: ( location ) => dispatch( metaBoxLoaded( location ) ),
};
}
Expand Down
6 changes: 4 additions & 2 deletions editor/components/post-saved-state/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ import {
isEditedPostSaveable,
getCurrentPost,
getEditedPostAttribute,
hasMetaBoxes,
} from '../../store/selectors';

export function PostSavedState( { isNew, isPublished, isDirty, isSaving, isSaveable, status, onStatusChange, onSave } ) {
export function PostSavedState( { hasActiveMetaboxes, isNew, isPublished, isDirty, isSaving, isSaveable, status, onStatusChange, onSave } ) {
const className = 'editor-post-saved-state';

if ( isSaving ) {
Expand All @@ -40,7 +41,7 @@ export function PostSavedState( { isNew, isPublished, isDirty, isSaving, isSavea
return null;
}

if ( ! isNew && ! isDirty ) {
if ( ! isNew && ! isDirty && ! hasActiveMetaboxes ) {
return (
<span className={ className }>
<Dashicon icon="saved" />
Expand Down Expand Up @@ -74,6 +75,7 @@ export default connect(
isSaving: isSavingPost( state ),
isSaveable: isEditedPostSaveable( state ),
status: getEditedPostAttribute( state, 'status' ),
hasActiveMetaboxes: hasMetaBoxes( state ),
} ),
{
onStatusChange: ( status ) => editPost( { status } ),
Expand Down
21 changes: 1 addition & 20 deletions editor/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -479,30 +479,11 @@ export function metaBoxLoaded( location ) {
/**
* Returns an action object used to request meta box update.
*
* @param {Array} locations Locations of meta boxes: ['normal', 'side', 'advanced' ].
*
* @return {Object} Action object
*/
export function requestMetaBoxUpdates( locations ) {
export function requestMetaBoxUpdates() {
return {
type: 'REQUEST_META_BOX_UPDATES',
locations,
};
}

/**
* Returns an action object used to set meta box state changed.
*
* @param {String} location Location of meta box: 'normal', 'side' or 'advanced'.
* @param {Boolean} hasChanged Whether the meta box has changed.
*
* @return {Object} Action object
*/
export function metaBoxStateChanged( location, hasChanged ) {
return {
type: 'META_BOX_STATE_CHANGED',
location,
hasChanged,
};
}

Expand Down
5 changes: 2 additions & 3 deletions editor/store/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {
import {
getCurrentPost,
getCurrentPostType,
getDirtyMetaBoxes,
getEditedPostContent,
getPostEdits,
isCurrentPostPublished,
Expand Down Expand Up @@ -105,7 +104,7 @@ export default {
},
REQUEST_POST_UPDATE_SUCCESS( action, store ) {
const { previousPost, post } = action;
const { dispatch, getState } = store;
const { dispatch } = store;

const publishStatus = [ 'publish', 'private', 'future' ];
const isPublished = includes( publishStatus, previousPost.status );
Expand Down Expand Up @@ -145,7 +144,7 @@ export default {
}

// Update dirty meta boxes.
dispatch( requestMetaBoxUpdates( getDirtyMetaBoxes( getState() ) ) );
dispatch( requestMetaBoxUpdates() );

if ( get( window.history.state, 'id' ) !== post.id ) {
window.history.replaceState(
Expand Down
23 changes: 5 additions & 18 deletions editor/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,6 @@ const locations = [
const defaultMetaBoxState = locations.reduce( ( result, key ) => {
result[ key ] = {
isActive: false,
isDirty: false,
isUpdating: false,
};

Expand All @@ -678,7 +677,6 @@ export function metaBoxes( state = defaultMetaBoxState, action ) {
...state[ action.location ],
isLoaded: true,
isUpdating: false,
isDirty: false,
},
};
case 'HANDLE_META_BOX_RELOAD':
Expand All @@ -687,26 +685,15 @@ export function metaBoxes( state = defaultMetaBoxState, action ) {
[ action.location ]: {
...state[ action.location ],
isUpdating: false,
isDirty: false,
},
};
case 'REQUEST_META_BOX_UPDATES':
return action.locations.reduce( ( newState, location ) => {
newState[ location ] = {
...state[ location ],
isUpdating: true,
isDirty: false,
return mapValues( state, ( metaBox ) => {
return {
...metaBox,
isUpdating: metaBox.isActive,
};
return newState;
}, { ...state } );
case 'META_BOX_STATE_CHANGED':
return {
...state,
[ action.location ]: {
...state[ action.location ],
isDirty: action.hasChanged,
},
};
} );
default:
return state;
}
Expand Down
39 changes: 21 additions & 18 deletions editor/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
without,
compact,
find,
some,
} from 'lodash';
import createSelector from 'rememo';

Expand Down Expand Up @@ -65,32 +66,34 @@ export function getMetaBox( state, location ) {
}

/**
* Returns a list of dirty meta box locations.
* Returns true if the post is using Meta Boxes
*
* @param {Object} state Global application state
* @return {Array} Array of locations for dirty meta boxes.
* @return {Boolean} Whether there are metaboxes or not.
*/
export const getDirtyMetaBoxes = createSelector(
export const hasMetaBoxes = createSelector(
( state ) => {
return reduce( getMetaBoxes( state ), ( result, metaBox, location ) => {
return metaBox.isDirty && metaBox.isActive ?
[ ...result, location ] :
result;
}, [] );
return some( getMetaBoxes( state ), ( metaBox ) => {
return metaBox.isActive;
} );
},
( state ) => state.metaBoxes,
);

/**
* Returns the dirty state of legacy meta boxes.
* Returns true if the the Meta Boxes are being saved
*
* Checks whether the entire meta box state is dirty. So if a sidebar is dirty,
* but a normal area is not dirty, this will overall return dirty.
*
* @param {Object} state Global application state
* @return {Boolean} Whether state is dirty. True if dirty, false if not.
* @param {Object} state Global application state
* @return {Boolean} Whether the metaboxes are being saved.
*/
export const isMetaBoxStateDirty = ( state ) => getDirtyMetaBoxes( state ).length > 0;
export const isSavingMetaBoxes = createSelector(
( state ) => {
return some( getMetaBoxes( state ), ( metaBox ) => {
return metaBox.isUpdating;
} );
},
( state ) => state.metaBoxes,
);

/**
* Returns the current active panel for the sidebar.
Expand Down Expand Up @@ -206,7 +209,7 @@ export function isEditedPostNew( state ) {
* @return {Boolean} Whether unsaved values exist
*/
export function isEditedPostDirty( state ) {
return state.editor.isDirty || isMetaBoxStateDirty( state );
return state.editor.isDirty;
}

/**
Expand Down Expand Up @@ -351,7 +354,7 @@ export function isCurrentPostPublished( state ) {
*/
export function isEditedPostPublishable( state ) {
const post = getCurrentPost( state );
return isEditedPostDirty( state ) || [ 'publish', 'private', 'future' ].indexOf( post.status ) === -1;
return isEditedPostDirty( state ) || hasMetaBoxes( state ) || [ 'publish', 'private', 'future' ].indexOf( post.status ) === -1;
}

/**
Expand Down Expand Up @@ -938,7 +941,7 @@ export function isBlockInsertionPointVisible( state ) {
* @return {Boolean} Whether post is being saved
*/
export function isSavingPost( state ) {
return state.saving.requesting;
return state.saving.requesting || isSavingMetaBoxes( state );
}

/**
Expand Down
14 changes: 1 addition & 13 deletions editor/store/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
stopTyping,
requestMetaBoxUpdates,
handleMetaBoxReload,
metaBoxStateChanged,
initializeMetaBoxState,
fetchReusableBlocks,
updateReusableBlock,
Expand Down Expand Up @@ -555,9 +554,8 @@ describe( 'actions', () => {

describe( 'requestMetaBoxUpdates', () => {
it( 'should return the REQUEST_META_BOX_UPDATES action', () => {
expect( requestMetaBoxUpdates( [ 'normal' ] ) ).toEqual( {
expect( requestMetaBoxUpdates() ).toEqual( {
type: 'REQUEST_META_BOX_UPDATES',
locations: [ 'normal' ],
} );
} );
} );
Expand All @@ -571,16 +569,6 @@ describe( 'actions', () => {
} );
} );

describe( 'metaBoxStateChanged', () => {
it( 'should return the META_BOX_STATE_CHANGED action with a hasChanged flag', () => {
expect( metaBoxStateChanged( 'normal', true ) ).toEqual( {
type: 'META_BOX_STATE_CHANGED',
location: 'normal',
hasChanged: true,
} );
} );
} );

describe( 'initializeMetaBoxState', () => {
it( 'should return the META_BOX_STATE_CHANGED action with a hasChanged flag', () => {
const metaBoxes = {
Expand Down
Loading

0 comments on commit 9eff87c

Please sign in to comment.