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

Create a helper hook for field validation #37004

Closed
3 tasks
joshuatf opened this issue Feb 28, 2023 · 3 comments · Fixed by #37196
Closed
3 tasks

Create a helper hook for field validation #37004

joshuatf opened this issue Feb 28, 2023 · 3 comments · Fixed by #37196
Assignees
Labels
plugin: woocommerce Issues related to the WooCommerce Core plugin.

Comments

@joshuatf
Copy link
Contributor

joshuatf commented Feb 28, 2023

Blocked by #37007, #37005

Validation in blocks currently works by locking the post. Locks are connected to a specific ID, so many locks can be added to prevent a post from being saved.

https://github.com/WordPress/gutenberg/blob/8a0eedeb85ba019f7179070f63447bb120e60308/docs/reference-guides/data/data-core-editor.md?plain=1#L1112

To make this easier, we should add some helper hooks around the locking for validation use cases. The hook might look something like this:

const useValidation = ( id, validate ) => {
    const { lockPostSaving } = useDispatch( 'core/editor' );

    if ( ! validate( entity ) ) {
        lockPostSaving( id );
    } else {
        unlockPostSaving( id );
    }
}

And then in our block, we could pass in our own validation logic and ID:

const validateTitle = ( product ) => {
    if ( product.title.length < 2 ) {
        return false;
    }

    return true;
}

useValidation( 'product/title', validateTitle );

We should carefully consider the flexibility of this solution and make sure that it works well for our use case. Some things to consider:

  • Does passing the entity work for all use cases?
  • Does this surface locks in a meaningful way? Is there a chance the editor remains accidentally locked?
  • Does this tie in well with error handling and render? Will we need another util for that?

Acceptance criteria

  • A validation hook that is reusable across all fields
  • Post saving should be locked when invalid
  • Should support async validation as well as sync
@joshuatf
Copy link
Contributor Author

Estimate: 2

@github-actions github-actions bot added the status: awaiting triage This is a newly created issue waiting for triage. label Feb 28, 2023
@jonathansadowski jonathansadowski added plugin: woocommerce Issues related to the WooCommerce Core plugin. status: prioritization and removed status: awaiting triage This is a newly created issue waiting for triage. labels Feb 28, 2023
@louwie17
Copy link
Contributor

louwie17 commented Mar 1, 2023

Hmmm I think we should punt this issue and just make use of the lockPostSaving functions for now within each block we create.
Maybe once we have 7-8 fields moved over to blocks we will have a better idea of what is needed for this and if it would be of benefit.
I am mostly scared that as we create this so early on we will drive ourselves into blockers in the future, where this will actually limit us. I would prefer if we use as many of the native Gutenberg functions for now and come back later to synchronize things if we notice trends.

A hook like this may also cause performance issues, if we are exposing the entire entity within the hook. This will mean the hook has to run on every entity update, instead of only when the desired block updates.
I know there will be cases for this like the slug and sku fields that might require validation when the name field changes.

@louwie17
Copy link
Contributor

louwie17 commented Mar 9, 2023

Re-added to the cycle after discussion during grooming session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants