-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Ensure that the autosave happens on empty posts in the native apps. #23233
Ensure that the autosave happens on empty posts in the native apps. #23233
Conversation
On the native apps, the publish button is managed by native code on the apps (non Gutenberg/JS code). In order to update the status of the publish button correctly we need to autosave even when there is no content, in order to send that information across the native code.
Size Change: +13 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels weird having isEditedPostSaveable() returning
true` always for native.
If it works better this way for us now we could pass it but this might hit us at some point in the future. It might be good to think about alternatives.
As I understand, this is basically to get the autosave message when the post became empty (user deleted last character). What about we detect when the post gets empty and trigger the autosave message manually?
I'm approving since it works for us now and we can improve on a different PR
This works for me as expected on iOS. I agree with @etoledom that it feels weird to have Also worth noting that Android doesn't disable the publish button as best I can tell, so this fix does not affect that behavior on Android. Also though, since Android does not save on exiting the editor, this change actually fixes an issue where if you delete the last block and then exit the post on Android, the native side would have never gotten notified about the new empty state, so the "saved" post would be from before the block was deleted. Not a big bug, but it is a bug and this does fix it. |
Sorry, can you elaborate on that one @mchowning ? I was under the impression that saving does happen upon exit on WPAndroid so, I'm probably missing the circumstances you refer to on this one. Thanks! |
👋 @hypest ! WPAndroid currently only saves the post via the autosave mechanism; in other words, on exiting the editor the "saved" post is what was saved during the last autosave. For this reason, it is actually possible to type-then-exit-a-post-very-quickly and to lose a small bit of the just-typed text. iOS handles this differently and does save the post on exiting the editor. Adding save-on-exit on Android is on my todo list, I just haven't had a chance to finish it up yet (ran into some issues where the save seemed to be bizarrely slow and I haven't had a chance to investigate that). The description and comments on my draft PR adding the save-on-exit on Android gives a lot of context around this. |
Thanks for the link to the parallel conversation around the fix Matt! |
I'm not sure, but I bet @malinajirka knows. 🙂 |
I'm little late to the party, sorry. My notifications for the gutenberg repo were not set-up correctly.
I believe we used to have save-on-exit. However, with the introduction of the reactive auto-save approach, this flow became obsolete. Technically there isn't any need to save-on-exit as the reactive auto-save always mirrors the changes from the UI to the model. The fact that we can lose changes from the last 500ms is a bug in the throttle implementation. As per the linked conversation we decided to implement save-on-exit instead of fixing the bug in the "throttle" implementation after all - however, it hasn't been implemented yet. |
Description
On the native apps, the publish button is managed by native code on
the apps (non Gutenberg/JS code). In order to update the status of the
publish button correctly, we need to autosave even when there is no
content, in order to send that information across the native code.
How has this been tested?
This can be tested using this WP-iOS PR: wordpress-mobile/WordPress-iOS#14329
Screenshots
Types of changes
Checklist: