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

Jump to bottom

Block Editor Form Validation #7369

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Block Editor Form Validation #7369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
925df23
Create useBlockEditor hook
JoryHogeveen Oct 14, 2024
9a77a79
Implement useBlockEditor as an effect to validation message changes
JoryHogeveen Oct 14, 2024
0f1b2ab
Fix valid status
JoryHogeveen Oct 14, 2024
f46f416
Use hook on init to properly set global vars
JoryHogeveen Oct 14, 2024
f353464
Do not return false and validate messages
JoryHogeveen Oct 15, 2024
524da67
Use same var structure as elsewhere in the codebase
JoryHogeveen Oct 15, 2024
e011867
Auto rebuild assets
JoryHogeveen Oct 15, 2024
d03267e
Check if block editor is loaded.
JoryHogeveen Oct 15, 2024
98c6e97
Auto rebuild assets
JoryHogeveen Oct 15, 2024
60d0aab
Add `meetsConditionalLogic` as a hook dependency
JoryHogeveen Oct 15, 2024
0205024
Auto rebuild assets
JoryHogeveen Oct 15, 2024
d83e886
Rebuild assets
sc0ttkclark Nov 17, 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
2 changes: 1 addition & 1 deletion ui/js/blocks/pods-blocks-api.min.asset.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"dependencies":["lodash","react","react-dom","wp-api-fetch","wp-autop","wp-block-editor","wp-blocks","wp-components","wp-compose","wp-date","wp-element","wp-i18n","wp-keycodes","wp-server-side-render","wp-url"],"version":"74e7329c36abb1a485a0"}
{"dependencies":["lodash","react","react-dom","wp-api-fetch","wp-autop","wp-block-editor","wp-blocks","wp-components","wp-compose","wp-date","wp-element","wp-i18n","wp-keycodes","wp-server-side-render","wp-url"],"version":"e4dd30ce893e9c448705"}
2 changes: 1 addition & 1 deletion ui/js/blocks/pods-blocks-api.min.js
View file
Open in desktop

Large diffs are not rendered by default.

18 changes: 1 addition & 17 deletions ui/js/dfv/pods-dfv.min.asset.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1 @@
<<<<<<< HEAD
<<<<<<< HEAD
<<<<<<< HEAD
<<<<<<< HEAD
{"dependencies":["lodash","moment","react","react-dom","react-jsx-runtime","regenerator-runtime","wp-api-fetch","wp-autop","wp-components","wp-compose","wp-data","wp-element","wp-hooks","wp-i18n","wp-keycodes","wp-plugins","wp-primitives","wp-url"],"version":"82c8aaf9e4ae1481502c"}
=======
{"dependencies":["lodash","moment","react","react-dom","react-jsx-runtime","regenerator-runtime","wp-api-fetch","wp-autop","wp-components","wp-compose","wp-data","wp-element","wp-hooks","wp-i18n","wp-keycodes","wp-plugins","wp-primitives","wp-url"],"version":"a5ca5125b2a394cb0bdb"}
>>>>>>> 2b4f0cdf9 (Rebuild assets)
=======
{"dependencies":["lodash","moment","react","react-dom","react-jsx-runtime","regenerator-runtime","wp-api-fetch","wp-autop","wp-components","wp-compose","wp-data","wp-element","wp-hooks","wp-i18n","wp-keycodes","wp-plugins","wp-primitives","wp-url"],"version":"6c3b89ffe8da2dcd1d1f"}
>>>>>>> cbd2c2484 (Auto rebuild assets)
=======
{"dependencies":["lodash","moment","react","react-dom","react-jsx-runtime","regenerator-runtime","wp-api-fetch","wp-autop","wp-components","wp-compose","wp-data","wp-element","wp-hooks","wp-i18n","wp-keycodes","wp-plugins","wp-primitives","wp-url"],"version":"7081b505becf5dac1be8"}
>>>>>>> 6ccfcd880 (Revamp wizard screens across all components)
=======
{"dependencies":["lodash","moment","react","react-dom","react-jsx-runtime","regenerator-runtime","wp-api-fetch","wp-autop","wp-components","wp-compose","wp-data","wp-element","wp-hooks","wp-i18n","wp-keycodes","wp-plugins","wp-primitives","wp-url"],"version":"02ffd778d1fc97a84bb4"}
>>>>>>> 9b36dedd1 (Auto rebuild assets)
{"dependencies":["lodash","moment","react","react-dom","react-jsx-runtime","regenerator-runtime","wp-api-fetch","wp-autop","wp-components","wp-compose","wp-data","wp-element","wp-hooks","wp-i18n","wp-keycodes","wp-plugins","wp-primitives","wp-url"],"version":"9dfe3c75280b0a2d29de"}
18 changes: 1 addition & 17 deletions ui/js/dfv/pods-dfv.min.js
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 19 additions & 3 deletions ui/js/dfv/src/components/field-wrapper/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import React, { useRef, useState } from 'react';
import React, { useEffect, useRef, useState } from 'react';
import { isEqual, uniq } from 'lodash';
import PropTypes from 'prop-types';

Expand Down Expand Up @@ -36,6 +36,7 @@ import { FIELD_PROP_TYPE_SHAPE } from 'dfv/src/config/prop-types';

import './field-wrapper.scss';
import RepeatableFieldList from './repeatable-field-list';
import useBlockEditor from '../../hooks/useBlockEditor';

export const FieldWrapper = ( props ) => {
const {
Expand Down Expand Up @@ -192,6 +193,21 @@ export const FieldWrapper = ( props ) => {
value
);

// Handle Block Editor save lock.
const blockEditor = useBlockEditor();
useEffect( () => {
if ( ! meetsConditionalLogic || ! validationMessages.length ) {
blockEditor.unlockPostSaving( `pods-field-${ name }` );
} else {
blockEditor.lockPostSaving( `pods-field-${ name }`, validationMessages, () => setHasBlurred( true ) );
}

// Unlock on unmount.
return () => {
blockEditor.unlockPostSaving( `pods-field-${ name }` );
};
}, [ validationMessages, meetsConditionalLogic ] );

// Don't render a field that hasn't had its dependencies met.
if ( ! meetsConditionalLogic ) {
return <span ref={ fieldRef } />;
Expand Down Expand Up @@ -225,7 +241,7 @@ export const FieldWrapper = ( props ) => {
allPodValues={ passAllPodValues ? allPodValues : undefined }
allPodFieldsMap={ passAllPodFieldsMap ? allPodFieldsMap : undefined }
setOptionValue={ setOptionValue }
isValid={ !! validationMessages.length }
isValid={ ! validationMessages.length }
addValidationRules={ addValidationRules }
setHasBlurred={ () => setHasBlurred( true ) }
fieldConfig={ field }
Expand Down Expand Up @@ -259,7 +275,7 @@ export const FieldWrapper = ( props ) => {
allPodValues={ allPodValues }
allPodFieldsMap={ allPodFieldsMap }
setValue={ setValue }
isValid={ !! validationMessages.length }
isValid={ ! validationMessages.length }
addValidationRules={ addValidationRules }
setHasBlurred={ () => setHasBlurred( true ) }
fieldConfig={ processedFieldConfig }
Expand Down
4 changes: 4 additions & 0 deletions ui/js/dfv/src/core/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@ import ConnectedFieldWrapper from 'dfv/src/components/connected-field-wrapper';
* Pods config
*/
import { FIELD_PROP_TYPE_SHAPE } from 'dfv/src/config/prop-types';
import useBlockEditor from '../hooks/useBlockEditor';

const App = ( {
storeKey,
} ) => {
const fieldsData = PodsDFVAPI._fieldDataByStoreKeyPrefix[ storeKey ];
const allPodFieldsMap = new Map( fieldsData.map( ( field ) => [ field.name, field ] ) );

// Initialize Pods Block Editor overrides if available.
useBlockEditor();

const fieldComponents = fieldsData.map( ( fieldData = {} ) => {
const {
directRender = false,
Expand Down
77 changes: 77 additions & 0 deletions ui/js/dfv/src/hooks/useBlockEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@

const useBlockEditor = () => {
const editorSelect = wp.data?.select( 'core/editor' );
const editorDispatch = wp.data?.dispatch( 'core/editor' );
const notices = wp.data?.dispatch( 'core/notices' );

// @todo Use hook instead of savePost override once stable.
if ( ! window.PodsBlockEditor && editorDispatch && editorDispatch.hasOwnProperty( 'savePost' ) ) {
// First init.
window.PodsBlockEditor = {
Copy link
Member Author

Choose a reason for hiding this comment

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

Optionally make this a class with private props and getters/setters. Maybe even an external tool like the API instead of a React hook.

Copy link
Member Author

Choose a reason for hiding this comment

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

Lets keep it like this since we should definitely change this code once a hook is available.
The added todo will inform any dev checking this code in that it's a temp solution.

// Store original.
savePost: editorDispatch.savePost,
messages: {},
callbacks: {},
};

// Override the current editor savePost function.
editorDispatch.savePost = async ( options ) => {
options = options || {};

const pbe = window.PodsBlockEditor;

if ( ! Object.values( pbe.messages ).length ) {
// eslint-disable-next-line no-undef
return pbe.savePost.apply( this, arguments );
}

return new Promise( function( resolve, reject ) {
// Bail early if is autosave or preview.
Copy link
Member Author

Choose a reason for hiding this comment

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

@sc0ttkclark Should we allow saving for a draft?

Copy link
Member

Choose a reason for hiding this comment

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

I'm on the fence here. Maybe we can make that an option in settings or a constant/hook. Default to allow saves on drafts perhaps.

Copy link
Member

Choose a reason for hiding this comment

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

But there's no way to draft by default when you have a post that's already published. So we'd need to distinguish between save and save draft.

Copy link
Member Author

@JoryHogeveen JoryHogeveen Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We can also leave it as-is and add it later if you want. I can quite easily fetch the status value and check that, however, it's hard to define what statuses should be allowed or not.
For ease of use (and clarity) it might be better to just validate by default for now.

EDIT: example

const status = editorSelect.getEditedPostAttribute('status');
if ( [ 'draft', 'trash', 'otherStatus' ].includes( status ) ) {
    // Run default
}
// OR
if ( SOME_USER_SETTING_ARRAY.includes( status ) ) {
    // Run default
}

if ( options.isAutosave || options.isPreview ) {
return resolve( 'Validation ignored (autosave).' );
}
for ( const fieldName in pbe.messages ) {
if ( pbe.messages.hasOwnProperty( fieldName ) ) {
pbe.messages[ fieldName ].forEach( function( message ) {
notices.createErrorNotice( 'Pods: ' + message, { id: fieldName, isDismissible: true } );
} );
}
editorDispatch?.lockPostSaving( fieldName );
}
for ( const fieldCallback in pbe.callbacks ) {
if ( pbe.callbacks.hasOwnProperty( fieldCallback ) && 'function' === typeof pbe.callbacks[ fieldCallback ] ) {
pbe.callbacks[ fieldCallback ]();
}
}
return reject( 'Pods validation failed' );
} );
};
}

return {
data: wp.data,
select: editorSelect,
dispatch: editorDispatch,
notices,
lockPostSaving: ( name, messages, callback ) => {
// @todo Use hook instead of savePost override once stable.
//wp.hooks.addFilter( 'editor.__unstablePreSavePost', 'editor', filter );

const pbe = window.PodsBlockEditor;
if ( messages.length ) {
pbe.messages[ name ] = messages;
pbe.callbacks[ name ] = callback;
}
},
unlockPostSaving: ( name ) => {
// @todo Use hook instead of savePost override once stable.
//wp.hooks.removeFilter( 'editor.__unstablePreSavePost', 'editor', filter );

delete window.PodsBlockEditor.messages[ name ];
delete window.PodsBlockEditor.callbacks[ name ];
editorDispatch?.unlockPostSaving( name );
},
};
};

export default useBlockEditor;