gutenberg icon indicating copy to clipboard operation
gutenberg copied to clipboard

Add an async `__unstablePreSavePost` hook; resolving with false prevents saving

Open adamsilverstein opened this issue 1 year ago • 3 comments

What?

  • Add a new experimental hook __unstablePreSavePost. resolving with false will prevent save from occurring.

Fixes https://github.com/WordPress/gutenberg/issues/13413

Why?

Useful for pre-save validation where a plugin wants to validate a set of requirements before allowing the post to save. Instead of trying to disable the save button itself, this hook provides a validation opportunity _right before the post save is actually triggered. When validation fails, plugins can add a visual indication of the issue in their UI and resolve to false here to prevent the save from occurring.

See https://github.com/WordPress/gutenberg/issues/13413 for additional discussion of use cases.

How?

  • Add a new async call to applyFilters in the savePost action right before the save actually occurs.
  • Returns early if the filter resolves to false, same as what happens when isEditedPostSaveable returning false

Testing Instructions

  1. Edit a page and click update, note save works as expected.
  2. Add the following code in the console which will prevent a save from happening (simulating some validation failing which I'm not indicating for simplicity): wp.hooks.addFilter('editor.__unstablePreSavePost', 'editor', function(){ return Promise.resolve(false); } );
  3. Try making a change and clicking Update - note a save is not triggered
  4. Add another filter that resolves to true: wp.hooks.removeAllFilters( 'editor.__unstablePreSavePost' );wp.hooks.addFilter('editor.__unstablePreSavePost', 'editor', function(){ return Promise.resolve(true); } );
  5. Try clicking the update button, it will work as expected

adamsilverstein avatar Jan 19 '24 18:01 adamsilverstein

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @aduth, @lex127, @audiovisuel-uqam, @eballeste, @jenilk, @zhitaryksergey, @christianMiguez.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: aduth, lex127, audiovisuel-uqam, eballeste, jenilk, zhitaryksergey, christianMiguez.

Co-authored-by: adamsilverstein <[email protected]>
Co-authored-by: jsnajdr <[email protected]>
Co-authored-by: ockham <[email protected]>
Co-authored-by: sadmansh <[email protected]>
Co-authored-by: sc0ttkclark <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: gziolo <[email protected]>
Co-authored-by: acafourek <[email protected]>
Co-authored-by: marcwieland95 <[email protected]>
Co-authored-by: margolisj <[email protected]>
Co-authored-by: noisysocks <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions[bot] avatar May 08 '24 19:05 github-actions[bot]

@jsnajdr Getting back to this after a break, thanks for the feedback - I like the idea of using an Error and of being able to modify the edits.

I applied your suggestions as I understood them, can you take another look?

adamsilverstein avatar May 08 '24 19:05 adamsilverstein

@jsnajdr can you take a look at this please when you have a moment?

sadmansh avatar May 13 '24 04:05 sadmansh

Hi @adamsilverstein :wave: Do you plan to work on this PR? It's very close to being finished and it would be nice to get it into WordPress 6.6. I could take it over from you and get it merged if you agree.

jsnajdr avatar Jun 04 '24 07:06 jsnajdr

Excited for this PR

sc0ttkclark avatar Jun 18 '24 14:06 sc0ttkclark

Hi @adamsilverstein 👋 Do you plan to work on this PR? It's very close to being finished and it would be nice to get it into WordPress 6.6. I could take it over from you and get it merged if you agree.

Thanks for the ping @jsnajdr (and @bph). I will get back to updating this soon!

adamsilverstein avatar Jul 16 '24 15:07 adamsilverstein

@jsnajdr I have tried to address all your feedback, please review again when you have a moment!

adamsilverstein avatar Jul 16 '24 21:07 adamsilverstein

I have updated the PR so functionality is restored and tests pass. Appreciate any reviews/feedback.

adamsilverstein avatar Aug 01 '24 18:08 adamsilverstein

Sorry for the late feedback, I only just saw this.

I might be missing something, but I don't think this should ship with the __unstable prefix. Background is that the project no longer ships new __experimental or __unstable APIs since this discussion resulted in some changes - https://make.wordpress.org/core/2022/08/10/proposal-stop-merging-experimental-apis-from-gutenberg-to-wordpress-core/. Efforts are now being made to deprecate APIs with those prefixes where it's possible.

Regardless, __unstable means not for external use, and it seems like the goal of this is for external use, so I don't think it should ship in a release with the prefix.

talldan avatar Aug 02 '24 09:08 talldan

Sure, let's remove the __unstable prefix. From both preSavePost and savePost hooks. How much time do we have? Does the no-__unstable rule apply for plugin releases, or only for Core releases?

jsnajdr avatar Aug 02 '24 10:08 jsnajdr

I'm proposing the stabilization and some followup changes in #64198.

jsnajdr avatar Aug 02 '24 10:08 jsnajdr

Sure, let's remove the __unstable prefix. From both preSavePost and savePost hooks. How much time do we have? Does the no-__unstable rule apply for plugin releases, or only for Core releases?

Thanks for spinning up a PR quickly. I'll give it a review.

It's mostly core releases that are a concern, though ideally it also wouldn't ship in a plugin release. If it does I think the usual rule is deprecate for two plugin versions and then remove. It's a bit of hassle that's usually better to avoid where possible.

Next plugin release (19.0) is on 14th August, with the RC on 7th August, so there should be time to rectify.

talldan avatar Aug 05 '24 09:08 talldan