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

If a more recent revision/autosave exists, store its state on editor setup #7945

Merged
merged 22 commits into from
Apr 18, 2019

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Jul 13, 2018

closes #7416

Attempts to resolve an issue whereby the editor is unaware of the existence of
a more recent autosave, resulting in attempts to perform an autosave fail.

Description

This PR results in autosave state being fetched when the editor loads (via a resolver). This request is preloaded (will require a core patch). Changes include:

  • Migration of autosave state from '@wordpress/editor' to '@wordpress/core-data'.
  • Addition of resolver for 'getAutosave' selector.
  • Disabling of the preview button until autosave state is fetched.
  • Deprecation of old selectors/actions

How has this been tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@talldan talldan added [Type] Bug An existing feature does not function as intended [Feature] Saving Related to saving functionality labels Jul 13, 2018
@talldan talldan self-assigned this Jul 13, 2018
@talldan talldan requested a review from aduth July 13, 2018 05:06
lib/client-assets.php Outdated Show resolved Hide resolved
@talldan talldan force-pushed the fix/initial-autosave-state branch from 73ed574 to 4c32e56 Compare July 20, 2018 06:15
@talldan

This comment has been minimized.

@talldan talldan force-pushed the fix/initial-autosave-state branch 2 times, most recently from f5f49f9 to 27b921b Compare July 23, 2018 05:08
@youknowriad
Copy link
Contributor

This seems an important bug to fix, but I'm moving to 3.8 for now as it doesn't seem ready.

@talldan talldan force-pushed the fix/initial-autosave-state branch 2 times, most recently from 40cb6ab to 847c699 Compare September 4, 2018 08:38
@talldan talldan added the [Status] In Progress Tracking issues with work in progress label Sep 6, 2018
@talldan talldan force-pushed the fix/initial-autosave-state branch 2 times, most recently from 68791dd to 2af62da Compare September 7, 2018 14:21
@talldan

This comment has been minimized.

@talldan

This comment has been minimized.

@talldan talldan force-pushed the fix/initial-autosave-state branch 2 times, most recently from 53a19b0 to 9e1a31b Compare September 25, 2018 17:49
@talldan

This comment has been minimized.

@talldan talldan added Needs Technical Feedback Needs testing from a developer perspective. and removed [Status] In Progress Tracking issues with work in progress labels Oct 11, 2018
@aduth
Copy link
Member

aduth commented Nov 8, 2018

There's quite a bit which has since been changed in affected code here, for which I'm largely to blame (both for the changes and neglecting to review this sooner). Let me know if there's anything I can do to help with bringing this back up to date.

@talldan talldan force-pushed the fix/initial-autosave-state branch from e44f855 to 8cb8fe3 Compare November 12, 2018 11:02
@talldan
Copy link
Contributor Author

talldan commented Nov 12, 2018

@aduth - I've rebased it. I ended up dropping a couple of commits that didn't seem relevant any more. They seem to have been fixed since.

My understanding is that if we want to introduce this, the PHP code will need to patched into core. Is that right?

@aduth
Copy link
Member

aduth commented Nov 12, 2018

Yes, and there may be some conflict to anticipate between this and the changes introduced to core as part of Trac#45194 (specifically 43833), which doesn't appear to have yet been ported back to the plugin (cc @imath @danielbachhuber).

@aduth aduth added the REST API Interaction Related to REST API label Nov 12, 2018
@danielbachhuber
Copy link
Member

the changes introduced to core as part of Trac#45194 (specifically 43833), which doesn't appear to have yet been ported back to the plugin

It's still a WIP on #4155

@talldan talldan force-pushed the fix/initial-autosave-state branch from 3f068e0 to aa0c408 Compare April 16, 2019 09:51
@talldan
Copy link
Contributor Author

talldan commented Apr 16, 2019

@aduth - have fixed that issue with receiveAutosaves. I've also updated the deprecation versions, so hopefully everything is good 👍

@aduth
Copy link
Member

aduth commented Apr 17, 2019

This request is preloaded (will require a core patch).

Is this still the case? I don't see any PHP file edits included.

Is it possible that due to an earlier rebase where we'd been relying on redundant code in client-assets.php to specify the preload paths, it was removed in a rebase?

If we need it, we should filter block_editor_preload_paths. And if it requires a core patch, we should follow-up with a corresponding Trac ticket.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Thanks for the patience in delayed reviews. Nice work on this one 👍

Consider my prior comment as far as whether we should be preloading this path. I don't consider it a blocker since the preloading is more an optimization, but I'll leave it to you whether we should include it.

@talldan
Copy link
Contributor Author

talldan commented Apr 18, 2019

Thanks so much for you guidance, @aduth! This one has been through a few iterations, but feels good to finally get it merged.

I'll work on some follow-ups for the preloading.

@talldan talldan merged commit bb8deba into master Apr 18, 2019
@talldan talldan deleted the fix/initial-autosave-state branch April 18, 2019 03:36
@noisysocks
Copy link
Member

10 months and 5 days—that must be a record! 😀👏

@talldan
Copy link
Contributor Author

talldan commented Apr 18, 2019

@aduth here's a ticket with a patch for adding the preloading:
https://core.trac.wordpress.org/ticket/46974

@aduth
Copy link
Member

aduth commented Apr 18, 2019

@aduth here's a ticket with a patch for adding the preloading:
https://core.trac.wordpress.org/ticket/46974

👍

Depending if you think it's important, we could also introduce a filter in the Gutenberg plugin ahead of this being patched in core.

daniloercoli added a commit that referenced this pull request Apr 18, 2019
…rnmobile/887-History-stack-is-not-empty-on-a-fresh-start-of-the-editor

* 'master' of https://github.com/WordPress/gutenberg:
  Reset embed mocks on every request, stop request to instagram (#15046)
  Refactor core blocks to have save and transforms in their own files (part 2) (#14899)
  Fix pullquote import (#15036)
  Refactor core blocks to have save and transforms in their own files (part 4) (#14903)
  Refactor core blocks to have save and transforms in their own files (part 3) (#14902)
  Refactor core blocks to have deprecated extracted to their ownf files (p.1) (#14979)
  Test transform from media to embed blocks (#13997)
  If a more recent revision/autosave exists, store its state on editor setup (#7945)
  chore(release): publish
@talldan talldan mentioned this pull request Apr 19, 2019
5 tasks
@talldan
Copy link
Contributor Author

talldan commented Apr 19, 2019

Depending if you think it's important, we could also introduce a filter in the Gutenberg plugin ahead of this being patched in core.

I'm not completely convinced it's needed, but I made a PR - #15067

@youknowriad youknowriad added this to the 5.6 (Gutenberg) milestone May 13, 2019
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Sep 14, 2019
With the introduction of WordPress/gutenberg#7945, the block editor requests autosave data when the editor is loaded. This can be optimized by preloading the request server-side and then passing the request data to the client using the preloading mechanism in editor-form-blocks.php.

Props: talldanwp, aduth.

git-svn-id: https://develop.svn.wordpress.org/trunk@46110 602fd350-edb4-49c9-b593-d223f7449a82
nylen pushed a commit to nylen/wordpress-develop-svn that referenced this pull request Sep 14, 2019
With the introduction of WordPress/gutenberg#7945, the block editor requests autosave data when the editor is loaded. This can be optimized by preloading the request server-side and then passing the request data to the client using the preloading mechanism in editor-form-blocks.php.

Props: talldanwp, aduth.

git-svn-id: https://develop.svn.wordpress.org/trunk@46110 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 14, 2019
With the introduction of WordPress/gutenberg#7945, the block editor requests autosave data when the editor is loaded. This can be optimized by preloading the request server-side and then passing the request data to the client using the preloading mechanism in editor-form-blocks.php.

Props: talldanwp, aduth.
Built from https://develop.svn.wordpress.org/trunk@46110


git-svn-id: http://core.svn.wordpress.org/trunk@45922 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this pull request Sep 14, 2019
With the introduction of WordPress/gutenberg#7945, the block editor requests autosave data when the editor is loaded. This can be optimized by preloading the request server-side and then passing the request data to the client using the preloading mechanism in editor-form-blocks.php.

Props: talldanwp, aduth.
Built from https://develop.svn.wordpress.org/trunk@46110


git-svn-id: https://core.svn.wordpress.org/trunk@45922 1a063a9b-81f0-0310-95a4-ce76da25c4cd
miya0001 pushed a commit to cjk4wp/wordpress that referenced this pull request Oct 26, 2019
With the introduction of WordPress/gutenberg#7945, the block editor requests autosave data when the editor is loaded. This can be optimized by preloading the request server-side and then passing the request data to the client using the preloading mechanism in editor-form-blocks.php.

Props: talldanwp, aduth.

git-svn-id: http://develop.svn.wordpress.org/trunk@46110 602fd350-edb4-49c9-b593-d223f7449a82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Saving Related to saving functionality REST API Interaction Related to REST API [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saving: Autosaveable does not account for autosave existing at start of editing
8 participants