VideoPress block: Limit synchronization of block attributes to when post has just been saved

[SiobhyB]

The web version of the block only syncs changes to the block's settings when a post has been manually saved (ref). However, there isn't currently a built-in way to check when a post has just been saved on iOS or Android.

To address this, I'm imagining that we would:

• Set up a listener that receives a message over the bridge when a post is saved on the native side.
• Either directly listen for the aforementioned message in the useSyncMedia hook or, alternatively, connect this to a native version of the isSavingPost selector.

When we have implemented a way to detect when a post has just been saved on native, we can then limit the VideoPress block's synchronization accordingly.

[fluiddot]

Either directly listen for the aforementioned message in the useSyncMedia hook or, alternatively, connect this to a native version of the isSavingPost selector.

@SiobhyB I understand that if we listen for the save post event in the useSyncMedia hook, it's expected that the network request would be made in the JS side, is this accurate? If so, I'm concerned about the case when the post is saved upon closing the editor, because the JS environment will be destroyed and we won't be able to run that code.

[SiobhyB]

@fluiddot, that's a good point. I'm looking at the code for Android here as well as that code's associated PR. I don't have any immediate bright ideas, but will keep thinking. 🤔

[fluiddot]

@fluiddot, that's a good point. I'm looking at the code for Android here as well as that code's associated PR. I don't have any immediate bright ideas, but will keep thinking. 🤔

Thanks for sharing those references, I didn't know about them 😅.

I spent some time thinking about the potential options and want to share the following two:

1 - Delegate close to the editor

Add a step to the close & save process that is initiated in the host apps. Before closing the editor, the app will notify the editor via the bridge of the intent of closing & saving. The editor will process this event and execute any pre-close code that is needed (e.g. saving the VideoPress block settings). Once the editor is ready, it will notify the app via the bridge that the editor can be closed.

The main downside of this approach is that we'll introduce a delay in the close & save flow due to potential asynchronous actions like saving the settings. In case the user has a slow connection, this would imply speeding down the close process. Although we could limit the execution duration of the pre-close actions to ensure that the editor is closed after X seconds.

2 - Delegate the save of VideoPress block settings to the save post logic in the host apps

Similarly to saving the post content, we'd need to provide a mechanism in the bridge to share VideoPress block settings with the host apps. The synchronization could happen when changing any of the settings or upon the close & save moment. With this data in the host apps, we'll update the settings during the save post flow that happens in the apps when closing.

This approach would be probably more complex and harder to maintain than the first one, as we need to integrate the synchronization mechanism. Plus, we'd have to make it generic as we don't want to reference WPCOM/VIdeoPress code in Gutenberg where the bridge is.

I'd advocate going with the first one due to simplicity, although it won't be trivial to implement and introduce an important change to the close & save flow. @SiobhyB WDYT?

[SiobhyB]

Thank you for sharing these options.

I'd advocate going with the first one due to simplicity, although it won't be trivial to implement and introduce an important change to the close & save flow. @SiobhyB WDYT?

Your reasoning makes sense to me here, and I agree that this seems the preferable option out of the two. I worry about the potential hit to performance, but think it's the best option we have and could potentially come in useful for other blocks or editor functionality in the future.

[fluiddot]

Your reasoning makes sense to me here, and I agree that this seems the preferable option out of the two. I worry about the potential hit to performance, but think it's the best option we have and could potentially come in useful for other blocks or editor functionality in the future.

Yes, that approach comes with a potential hit to performance. Ideally, the update should happen in the host apps after the editor is closed like we could on the second option.

I'm thinking of a third option that combines the two mentioned above. It involves sending extra data to be saved when closing & saving. What I have in mind is the following:

1. The VideoPress block listens to an event that will be triggered upon closing the editor.
2. App notifies the editor about the closing. The editor triggers the above-mentioned event.
3. The VideoPress block gets notified and responds to the event with the settings that need to be saved, e.g.:

{
  videopress_data_to_update: { title: "New title", rating: "R" },
}

4. The editor collects all the data from the event and creates a new object:

{
  other_block_data_to_update: {},
  ...
  videopress_data_to_update: { title: "New title", rating: "R" },
}

5. The editor notifies back to the host app with the data that the editor can be closed.
6. As part of the save post process, the above data passed will be processed individually. In the case of the VideoPress data, the app will hit the metadata endpoint.

[SiobhyB]

@fluiddot, I've spent some time looking into how changes are saved to Gutenberg upon exiting the editor on Android, with the following resources being what I've relied on the most:

• Save latest post changes when exiting editor WordPress-Android#12144
• p9ugOq-Hx-p2
• p9ugOq-1Jh-p2

As per this code comment:

Ideally, there should never be any changes that need to be saved when exiting the editor because all changes should be caught by the autosave mechanism, but there is a known issue where there will be unsaved changes when the user quickly exits the editor after making changes.

We appear to heavily rely on the information we get from autosaving when exiting the editor.

If I'm understanding correctly, I believe a user would only be able to exit the editor quickly enough to cause issues after directly typing. If they open up a block's settings and make changes, they have to close the settings sheet before exiting, which would provide the necessary time for autosave to do its job. We could likely have solid confidence in the autosave mechanism catching changes users make to the block's settings before they exit the editor.

As we're already relying on the autosave mechanism for saving on exit, I wonder if we could simplify our approach to syncing the VideoPress block's setting by relying on those existing autosave methods.

You mentioned at p1681400912036929/1681327194.576099-slack-C04LWMHSGLC that the web also appears to be updating on autosave, so I feel like this could be an acceptable approach and one that wouldn't involve more complex solutions or performance hits.

What do you think?

[fluiddot]

We appear to heavily rely on the information we get from autosaving when exiting the editor.

If I'm understanding correctly, I believe a user would only be able to exit the editor quickly enough to cause issues after directly typing. If they open up a block's settings and make changes, they have to close the settings sheet before exiting, which would provide the necessary time for autosave to do its job. We could likely have solid confidence in the autosave mechanism catching changes users make to the block's settings before they exit the editor.

That's interesting. I haven't tested it myself but if the autosave is triggered quickly after closing the settings, we could rely on this behavior to save settings to VideoPress.

As we're already relying on the autosave mechanism for saving on exit, I wonder if we could simplify our approach to syncing the VideoPress block's setting by relying on those existing autosave methods.

Good point. If the content that we save when exiting is actually the one made via the autosave mechanism, makes sense to use this approach also to sync VideoPress settings.

You mentioned at p1681400912036929/1681327194.576099-slack-C04LWMHSGLC that the web also appears to be updating on autosave, so I feel like this could be an acceptable approach and one that wouldn't involve more complex solutions or performance hits.
What do you think?

I agree, with the information you shared seems the autosave is a way more reliable mechanism than I thought for saving content 😅. Being this said, let's go with this approach 🚀 .

[SiobhyB]

Thanks for the confidence check @fluiddot! I can aim to get a proof of concept ready for review on Monday 🙇‍♀️ 🤞

[fluiddot]

Thanks for the confidence check @fluiddot! I can aim to get a proof of concept ready for review on Monday 🙇‍♀️ 🤞

@SiobhyB let me know if I can help anyhow with the proof of concept. I saw you made some progress in #5663 and its companion PRs, but not sure what's the optimal way to divide up the work.

[SiobhyB]

@fluiddot, thank you so much, I'm just working on the iOS PR now and hope to have something that's ready for review in the next couple of hours. I don't think there's a great way to divide the development work right now, but would be grateful if you could help to review when it's ready 🙇‍♀️

[fluiddot]

@fluiddot, thank you so much, I'm just working on the iOS PR now and hope to have something that's ready for review in the next couple of hours. I don't think there's a great way to divide the development work right now, but would be grateful if you could help to review when it's ready 🙇‍♀️

Sure thing, I'm more than happy to contribute by reviewing the PRs 🙂.

[SiobhyB]

Noting that upon further experimentation, I found it was possible to get the syncing working upon a manual save without the need for any super complicated solutions. The solution in #5663 works in my testing, even for flows where the user exits the editor. 🎉

The autosave approach began to feel like too much, given the Android app autosaves after any additional text is added to the editor, so the proposed solution in my PR works only when a user manually saves a post. Looking forward to hearing any feedback on this. 🙇‍♀️

[fluiddot]

The solution in #5663 works in my testing, even for flows where the user exits the editor. 🎉

@SiobhyB That's great! I noticed that in case the editor is closed, we make the POST request to update the metadata right before closing. It's likely that the editor will be closed at the time the request finishes, so we won't execute this code. However, I think this is no a critical issue as we only want to update the metadata in the native version 👍 . In the future, if we need to upload the autogenerated tracks (reference) after saving the metadata, we'd need to figure out how to do it.

The autosave approach began to feel like too much, given the Android app autosaves after any additional text is added to the editor, so the proposed solution in my PR works only when a user manually saves a post. Looking forward to hearing any feedback on this. 🙇‍♀️

Sounds good. I think saving only when saving a post makes sense.