Sync unsaved changes before leaving

[MisRob]

Description

Inform users about unsaved changes and sync them before they leave the application.

Issue Addressed

Closes#2496

Steps to Test

• Increase SYNC_IF_NO_CHANGES_FOR, for example to 20 seconds, so you're able to leave the application comfortably in all possible ways
• Make some updates that use /sync endpoint, for example add a topic to a channel in the channel editor or update a node in the edit modal
• Leave the app shortly after that by closing a window or logging out

Implementation Notes

This PR adds SyncProgressPlugin Vuex plugin that updates unsavedChanges to true/false based on whether there are some unsaved changes in the IndexedDB table where changes for sync are kept. The value will be updated on each change of the table. The plugin is installed during an app initialization.

Leaving by closing a window

During an app initialization, beforeunload handler is defined. The handler runs server sync immediately and prevents unload if there are any unsavedChanges by showing the default browser prompt to a user who can then decide if they really want to leave while still having some unsaved changes.

Leaving by using logout function

When logging out, a custom confirmation dialog is displayed if there are any unsavedChanges and server sync is triggered immediately. If user confirms that they want to leave, logout will be performed and additionally, logoutConfirmed:true will be saved to sessionStorage. logoutConfirmed value is then checked in beforeunload handler and if set to true, the handler won't continue executing. This is to prevent from showing a confirmation dialog the second time in case a user has already confirmed that they want to leave.

---

The main reason behind this implementation is that beforeunload event handler has some limits to prevent developers from introducing malicious functionality. One of those limitations is that decision logic related to preventing unload needs to be synchronous. Because of that, it doesn't seem to be possible to reuse the solution from SavingIndicator .

Checklist

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?

Design notes

Even though it was possible to specify a custom message in the past in beforeunload event handler, most modern browsers have dropped support. Therefore a default browser message (can vary across browsers) will be displayed when leaving the app by closing a window. Because of that, the message that @jtamiacedesigned is used only in a prompt displayed after trying to log out in a regular way.

Comments

I'd appreciate some thoughts on the following:

Saving indicator

I was thinking about connecting SavingIndicator used in edit modal toSyncProgressPlugin instead of polling IndexedDB though at this point, SavingIndicator is implemented to check only for saving progress of nodes being edited whereas unsavedChanges is simply a Boolean value containing information about whether there are any unsaved changes from any part of the app.

Possible solutions:

(1) Leave it as is
(2) Implement SyncProgressPlugin in a much more granular way to contain information about nodes etc.
(3) Simplify SavingIndicator to check whether there are any unsaved changes by calling areAllChangesSaved getter, not only those related to nodes being currently edited in the edit modal.

I like consistency of (3) and it seems to make sense from UX point of view because with approaches (1) and (2), we could run into a situation when saving indicator would not visible but "Unsaved changes" prompt could be still displayed when trying to exit the app while having some unsaved changes related to nodes that are currently not being edited. Thoughts?

Navigation between apps

Due to our multiple apps architecture, the "Unsaved changes" prompt can be displayed when navigating between apps when there are unsaved changes which disrupts user experience. I increased SYNC_IF_NO_CHANGES_FOR to 20 seconds for the sake of testing this PR so you can check this for example by adding a new topic in a channel editor and navigating to /channels URL shortly after that.

There is no information on destination URL in beforeunload event (again, to prevent from malicious use), so I am not sure how to solve it. I think there is no need to be worried about it because our default value for SYNC_IF_NO_CHANGES_FOR is 2 seconds so it shouldn't happen very often and also it will be solved for free as soon as we migrate to using one app. Is it okay for now?

[MisRob]

TODO: Remove "Temporarily increase sync interval" commit before merging Done

[codecov]

Codecov Report

Merging #2699 (4a61f9a) into develop (c767412) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## develop #2699 +/- ##
========================================
Coverage 84.94% 84.94% ========================================
Files 301 301 Lines 15227 15227 ========================================
Hits 12934 12934 Misses 2293 2293 

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44518a3...5b5ecab. Read the comment docs.

[rtibbles]

The logout flow seems to be mostly covered, but this might cause aborted sync requests in both the logout and non-logout navigating away from the page flows.

[rtibbles]

Flagging that this won't be translated just yet.

[rtibbles]

Manual testing checks out.

[MisRob]

Description

Inform users about unsaved changes and sync them before they leave the application.

Issue Addressed

Closes#2496

Steps to Test

• Increase SYNC_IF_NO_CHANGES_FOR, for example to 20 seconds, so you're able to leave the application comfortably in all possible ways
• Make some updates that use /sync endpoint, for example add a topic to a channel in the channel editor or update a node in the edit modal
• Leave the app shortly after that by closing a window or logging out

Implementation Notes

This PR adds SyncProgressPlugin Vuex plugin that updates unsavedChanges to true/false based on whether there are some unsaved changes in the IndexedDB table where changes for sync are kept. The value will be updated on each change of the table. The plugin is installed during an app initialization.

Leaving by closing a window

During an app initialization, beforeunload handler is defined. The handler runs server sync immediately and prevents unload if there are any unsavedChanges by showing the default browser prompt to a user who can then decide if they really want to leave while still having some unsaved changes.

Leaving by using logout function

When logging out, a custom confirmation dialog is displayed if there are any unsavedChanges and server sync is triggered immediately. If user confirms that they want to leave, logout will be performed and additionally, logoutConfirmed:true will be saved to sessionStorage. logoutConfirmed value is then checked in beforeunload handler and if set to true, the handler won't continue executing. This is to prevent from showing a confirmation dialog the second time in case a user has already confirmed that they want to leave.

---

The main reason behind this implementation is that beforeunload event handler has some limits to prevent developers from introducing malicious functionality. One of those limitations is that decision logic related to preventing unload needs to be synchronous. Because of that, it doesn't seem to be possible to reuse the solution from SavingIndicator .

Checklist

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?

Design notes

Even though it was possible to specify a custom message in the past in beforeunload event handler, most modern browsers have dropped support. Therefore a default browser message (can vary across browsers) will be displayed when leaving the app by closing a window. Because of that, the message that @jtamiacedesigned is used only in a prompt displayed after trying to log out in a regular way.

Comments

I'd appreciate some thoughts on the following:

Saving indicator

I was thinking about connecting SavingIndicator used in edit modal toSyncProgressPlugin instead of polling IndexedDB though at this point, SavingIndicator is implemented to check only for saving progress of nodes being edited whereas unsavedChanges is simply a Boolean value containing information about whether there are any unsaved changes from any part of the app.

Possible solutions:

(1) Leave it as is
(2) Implement SyncProgressPlugin in a much more granular way to contain information about nodes etc.
(3) Simplify SavingIndicator to check whether there are any unsaved changes by calling areAllChangesSaved getter, not only those related to nodes being currently edited in the edit modal.

I like consistency of (3) and it seems to make sense from UX point of view because with approaches (1) and (2), we could run into a situation when saving indicator would not visible but "Unsaved changes" prompt could be still displayed when trying to exit the app while having some unsaved changes related to nodes that are currently not being edited. Thoughts?

Navigation between apps

Due to our multiple apps architecture, the "Unsaved changes" prompt can be displayed when navigating between apps when there are unsaved changes which disrupts user experience. I increased SYNC_IF_NO_CHANGES_FOR to 20 seconds for the sake of testing this PR so you can check this for example by adding a new topic in a channel editor and navigating to /channels URL shortly after that.

There is no information on destination URL in beforeunload event (again, to prevent from malicious use), so I am not sure how to solve it. I think there is no need to be worried about it because our default value for SYNC_IF_NO_CHANGES_FOR is 2 seconds so it shouldn't happen very often and also it will be solved for free as soon as we migrate to using one app. Is it okay for now?

[MisRob]

TODO: Remove "Temporarily increase sync interval" commit before merging Done

[codecov]

Codecov Report

Merging #2699 (4a61f9a) into develop (c767412) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## develop #2699 +/- ##
========================================
Coverage 84.94% 84.94% ========================================
Files 301 301 Lines 15227 15227 ========================================
Hits 12934 12934 Misses 2293 2293 

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44518a3...5b5ecab. Read the comment docs.

[rtibbles]

The logout flow seems to be mostly covered, but this might cause aborted sync requests in both the logout and non-logout navigating away from the page flows.

[rtibbles]

Flagging that this won't be translated just yet.

[rtibbles]

Manual testing checks out.

[MisRob]

Description

Inform users about unsaved changes and sync them before they leave the application.

Issue Addressed

Closes#2496

Steps to Test

• Increase SYNC_IF_NO_CHANGES_FOR, for example to 20 seconds, so you're able to leave the application comfortably in all possible ways
• Make some updates that use /sync endpoint, for example add a topic to a channel in the channel editor or update a node in the edit modal
• Leave the app shortly after that by closing a window or logging out

Implementation Notes

This PR adds SyncProgressPlugin Vuex plugin that updates unsavedChanges to true/false based on whether there are some unsaved changes in the IndexedDB table where changes for sync are kept. The value will be updated on each change of the table. The plugin is installed during an app initialization.

Leaving by closing a window

During an app initialization, beforeunload handler is defined. The handler runs server sync immediately and prevents unload if there are any unsavedChanges by showing the default browser prompt to a user who can then decide if they really want to leave while still having some unsaved changes.

Leaving by using logout function

When logging out, a custom confirmation dialog is displayed if there are any unsavedChanges and server sync is triggered immediately. If user confirms that they want to leave, logout will be performed and additionally, logoutConfirmed:true will be saved to sessionStorage. logoutConfirmed value is then checked in beforeunload handler and if set to true, the handler won't continue executing. This is to prevent from showing a confirmation dialog the second time in case a user has already confirmed that they want to leave.

---

The main reason behind this implementation is that beforeunload event handler has some limits to prevent developers from introducing malicious functionality. One of those limitations is that decision logic related to preventing unload needs to be synchronous. Because of that, it doesn't seem to be possible to reuse the solution from SavingIndicator .

Checklist

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?

Design notes

Even though it was possible to specify a custom message in the past in beforeunload event handler, most modern browsers have dropped support. Therefore a default browser message (can vary across browsers) will be displayed when leaving the app by closing a window. Because of that, the message that @jtamiacedesigned is used only in a prompt displayed after trying to log out in a regular way.

Comments

I'd appreciate some thoughts on the following:

Saving indicator

I was thinking about connecting SavingIndicator used in edit modal toSyncProgressPlugin instead of polling IndexedDB though at this point, SavingIndicator is implemented to check only for saving progress of nodes being edited whereas unsavedChanges is simply a Boolean value containing information about whether there are any unsaved changes from any part of the app.

Possible solutions:

(1) Leave it as is
(2) Implement SyncProgressPlugin in a much more granular way to contain information about nodes etc.
(3) Simplify SavingIndicator to check whether there are any unsaved changes by calling areAllChangesSaved getter, not only those related to nodes being currently edited in the edit modal.

I like consistency of (3) and it seems to make sense from UX point of view because with approaches (1) and (2), we could run into a situation when saving indicator would not visible but "Unsaved changes" prompt could be still displayed when trying to exit the app while having some unsaved changes related to nodes that are currently not being edited. Thoughts?

Navigation between apps

Due to our multiple apps architecture, the "Unsaved changes" prompt can be displayed when navigating between apps when there are unsaved changes which disrupts user experience. I increased SYNC_IF_NO_CHANGES_FOR to 20 seconds for the sake of testing this PR so you can check this for example by adding a new topic in a channel editor and navigating to /channels URL shortly after that.

There is no information on destination URL in beforeunload event (again, to prevent from malicious use), so I am not sure how to solve it. I think there is no need to be worried about it because our default value for SYNC_IF_NO_CHANGES_FOR is 2 seconds so it shouldn't happen very often and also it will be solved for free as soon as we migrate to using one app. Is it okay for now?

[MisRob]

TODO: Remove "Temporarily increase sync interval" commit before merging Done

[codecov]

Codecov Report

Merging #2699 (4a61f9a) into develop (c767412) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## develop #2699 +/- ##
========================================
Coverage 84.94% 84.94% ========================================
Files 301 301 Lines 15227 15227 ========================================
Hits 12934 12934 Misses 2293 2293 

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44518a3...5b5ecab. Read the comment docs.

[rtibbles]

The logout flow seems to be mostly covered, but this might cause aborted sync requests in both the logout and non-logout navigating away from the page flows.

[rtibbles]

Flagging that this won't be translated just yet.

[rtibbles]

Manual testing checks out.

[MisRob]

Description

Inform users about unsaved changes and sync them before they leave the application.

Issue Addressed

Closes#2496

Steps to Test

• Increase SYNC_IF_NO_CHANGES_FOR, for example to 20 seconds, so you're able to leave the application comfortably in all possible ways
• Make some updates that use /sync endpoint, for example add a topic to a channel in the channel editor or update a node in the edit modal
• Leave the app shortly after that by closing a window or logging out

Implementation Notes

This PR adds SyncProgressPlugin Vuex plugin that updates unsavedChanges to true/false based on whether there are some unsaved changes in the IndexedDB table where changes for sync are kept. The value will be updated on each change of the table. The plugin is installed during an app initialization.

Leaving by closing a window

During an app initialization, beforeunload handler is defined. The handler runs server sync immediately and prevents unload if there are any unsavedChanges by showing the default browser prompt to a user who can then decide if they really want to leave while still having some unsaved changes.

Leaving by using logout function

When logging out, a custom confirmation dialog is displayed if there are any unsavedChanges and server sync is triggered immediately. If user confirms that they want to leave, logout will be performed and additionally, logoutConfirmed:true will be saved to sessionStorage. logoutConfirmed value is then checked in beforeunload handler and if set to true, the handler won't continue executing. This is to prevent from showing a confirmation dialog the second time in case a user has already confirmed that they want to leave.

---

The main reason behind this implementation is that beforeunload event handler has some limits to prevent developers from introducing malicious functionality. One of those limitations is that decision logic related to preventing unload needs to be synchronous. Because of that, it doesn't seem to be possible to reuse the solution from SavingIndicator .

Checklist

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?

Design notes

Even though it was possible to specify a custom message in the past in beforeunload event handler, most modern browsers have dropped support. Therefore a default browser message (can vary across browsers) will be displayed when leaving the app by closing a window. Because of that, the message that @jtamiacedesigned is used only in a prompt displayed after trying to log out in a regular way.

Comments

I'd appreciate some thoughts on the following:

Saving indicator

I was thinking about connecting SavingIndicator used in edit modal toSyncProgressPlugin instead of polling IndexedDB though at this point, SavingIndicator is implemented to check only for saving progress of nodes being edited whereas unsavedChanges is simply a Boolean value containing information about whether there are any unsaved changes from any part of the app.

Possible solutions:

(1) Leave it as is
(2) Implement SyncProgressPlugin in a much more granular way to contain information about nodes etc.
(3) Simplify SavingIndicator to check whether there are any unsaved changes by calling areAllChangesSaved getter, not only those related to nodes being currently edited in the edit modal.

I like consistency of (3) and it seems to make sense from UX point of view because with approaches (1) and (2), we could run into a situation when saving indicator would not visible but "Unsaved changes" prompt could be still displayed when trying to exit the app while having some unsaved changes related to nodes that are currently not being edited. Thoughts?

Navigation between apps

Due to our multiple apps architecture, the "Unsaved changes" prompt can be displayed when navigating between apps when there are unsaved changes which disrupts user experience. I increased SYNC_IF_NO_CHANGES_FOR to 20 seconds for the sake of testing this PR so you can check this for example by adding a new topic in a channel editor and navigating to /channels URL shortly after that.

There is no information on destination URL in beforeunload event (again, to prevent from malicious use), so I am not sure how to solve it. I think there is no need to be worried about it because our default value for SYNC_IF_NO_CHANGES_FOR is 2 seconds so it shouldn't happen very often and also it will be solved for free as soon as we migrate to using one app. Is it okay for now?

[MisRob]

TODO: Remove "Temporarily increase sync interval" commit before merging Done

[codecov]

Codecov Report

Merging #2699 (4a61f9a) into develop (c767412) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## develop #2699 +/- ##
========================================
Coverage 84.94% 84.94% ========================================
Files 301 301 Lines 15227 15227 ========================================
Hits 12934 12934 Misses 2293 2293 

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44518a3...5b5ecab. Read the comment docs.

[rtibbles]

The logout flow seems to be mostly covered, but this might cause aborted sync requests in both the logout and non-logout navigating away from the page flows.

[rtibbles]

Flagging that this won't be translated just yet.

[rtibbles]

Manual testing checks out.

[MisRob]

Description

Inform users about unsaved changes and sync them before they leave the application.

Issue Addressed

Closes#2496

Steps to Test

• Increase SYNC_IF_NO_CHANGES_FOR, for example to 20 seconds, so you're able to leave the application comfortably in all possible ways
• Make some updates that use /sync endpoint, for example add a topic to a channel in the channel editor or update a node in the edit modal
• Leave the app shortly after that by closing a window or logging out

Implementation Notes

This PR adds SyncProgressPlugin Vuex plugin that updates unsavedChanges to true/false based on whether there are some unsaved changes in the IndexedDB table where changes for sync are kept. The value will be updated on each change of the table. The plugin is installed during an app initialization.

Leaving by closing a window

During an app initialization, beforeunload handler is defined. The handler runs server sync immediately and prevents unload if there are any unsavedChanges by showing the default browser prompt to a user who can then decide if they really want to leave while still having some unsaved changes.

Leaving by using logout function

When logging out, a custom confirmation dialog is displayed if there are any unsavedChanges and server sync is triggered immediately. If user confirms that they want to leave, logout will be performed and additionally, logoutConfirmed:true will be saved to sessionStorage. logoutConfirmed value is then checked in beforeunload handler and if set to true, the handler won't continue executing. This is to prevent from showing a confirmation dialog the second time in case a user has already confirmed that they want to leave.

---

The main reason behind this implementation is that beforeunload event handler has some limits to prevent developers from introducing malicious functionality. One of those limitations is that decision logic related to preventing unload needs to be synchronous. Because of that, it doesn't seem to be possible to reuse the solution from SavingIndicator .

Checklist

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?

Design notes

Even though it was possible to specify a custom message in the past in beforeunload event handler, most modern browsers have dropped support. Therefore a default browser message (can vary across browsers) will be displayed when leaving the app by closing a window. Because of that, the message that @jtamiacedesigned is used only in a prompt displayed after trying to log out in a regular way.

Comments

I'd appreciate some thoughts on the following:

Saving indicator

I was thinking about connecting SavingIndicator used in edit modal toSyncProgressPlugin instead of polling IndexedDB though at this point, SavingIndicator is implemented to check only for saving progress of nodes being edited whereas unsavedChanges is simply a Boolean value containing information about whether there are any unsaved changes from any part of the app.

Possible solutions:

(1) Leave it as is
(2) Implement SyncProgressPlugin in a much more granular way to contain information about nodes etc.
(3) Simplify SavingIndicator to check whether there are any unsaved changes by calling areAllChangesSaved getter, not only those related to nodes being currently edited in the edit modal.

I like consistency of (3) and it seems to make sense from UX point of view because with approaches (1) and (2), we could run into a situation when saving indicator would not visible but "Unsaved changes" prompt could be still displayed when trying to exit the app while having some unsaved changes related to nodes that are currently not being edited. Thoughts?

Navigation between apps

Due to our multiple apps architecture, the "Unsaved changes" prompt can be displayed when navigating between apps when there are unsaved changes which disrupts user experience. I increased SYNC_IF_NO_CHANGES_FOR to 20 seconds for the sake of testing this PR so you can check this for example by adding a new topic in a channel editor and navigating to /channels URL shortly after that.

There is no information on destination URL in beforeunload event (again, to prevent from malicious use), so I am not sure how to solve it. I think there is no need to be worried about it because our default value for SYNC_IF_NO_CHANGES_FOR is 2 seconds so it shouldn't happen very often and also it will be solved for free as soon as we migrate to using one app. Is it okay for now?

[MisRob]

TODO: Remove "Temporarily increase sync interval" commit before merging Done

[codecov]

Codecov Report

Merging #2699 (4a61f9a) into develop (c767412) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## develop #2699 +/- ##
========================================
Coverage 84.94% 84.94% ========================================
Files 301 301 Lines 15227 15227 ========================================
Hits 12934 12934 Misses 2293 2293 

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44518a3...5b5ecab. Read the comment docs.

[rtibbles]

The logout flow seems to be mostly covered, but this might cause aborted sync requests in both the logout and non-logout navigating away from the page flows.

[rtibbles]

Flagging that this won't be translated just yet.

[rtibbles]

Manual testing checks out.

[MisRob]

Description

Inform users about unsaved changes and sync them before they leave the application.

Issue Addressed

Closes#2496

Steps to Test

• Increase SYNC_IF_NO_CHANGES_FOR, for example to 20 seconds, so you're able to leave the application comfortably in all possible ways
• Make some updates that use /sync endpoint, for example add a topic to a channel in the channel editor or update a node in the edit modal
• Leave the app shortly after that by closing a window or logging out

Implementation Notes

This PR adds SyncProgressPlugin Vuex plugin that updates unsavedChanges to true/false based on whether there are some unsaved changes in the IndexedDB table where changes for sync are kept. The value will be updated on each change of the table. The plugin is installed during an app initialization.

Leaving by closing a window

During an app initialization, beforeunload handler is defined. The handler runs server sync immediately and prevents unload if there are any unsavedChanges by showing the default browser prompt to a user who can then decide if they really want to leave while still having some unsaved changes.

Leaving by using logout function

When logging out, a custom confirmation dialog is displayed if there are any unsavedChanges and server sync is triggered immediately. If user confirms that they want to leave, logout will be performed and additionally, logoutConfirmed:true will be saved to sessionStorage. logoutConfirmed value is then checked in beforeunload handler and if set to true, the handler won't continue executing. This is to prevent from showing a confirmation dialog the second time in case a user has already confirmed that they want to leave.

---

The main reason behind this implementation is that beforeunload event handler has some limits to prevent developers from introducing malicious functionality. One of those limitations is that decision logic related to preventing unload needs to be synchronous. Because of that, it doesn't seem to be possible to reuse the solution from SavingIndicator .

Checklist

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?

Design notes

Even though it was possible to specify a custom message in the past in beforeunload event handler, most modern browsers have dropped support. Therefore a default browser message (can vary across browsers) will be displayed when leaving the app by closing a window. Because of that, the message that @jtamiacedesigned is used only in a prompt displayed after trying to log out in a regular way.

Comments

I'd appreciate some thoughts on the following:

Saving indicator

I was thinking about connecting SavingIndicator used in edit modal toSyncProgressPlugin instead of polling IndexedDB though at this point, SavingIndicator is implemented to check only for saving progress of nodes being edited whereas unsavedChanges is simply a Boolean value containing information about whether there are any unsaved changes from any part of the app.

Possible solutions:

(1) Leave it as is
(2) Implement SyncProgressPlugin in a much more granular way to contain information about nodes etc.
(3) Simplify SavingIndicator to check whether there are any unsaved changes by calling areAllChangesSaved getter, not only those related to nodes being currently edited in the edit modal.

I like consistency of (3) and it seems to make sense from UX point of view because with approaches (1) and (2), we could run into a situation when saving indicator would not visible but "Unsaved changes" prompt could be still displayed when trying to exit the app while having some unsaved changes related to nodes that are currently not being edited. Thoughts?

Navigation between apps

Due to our multiple apps architecture, the "Unsaved changes" prompt can be displayed when navigating between apps when there are unsaved changes which disrupts user experience. I increased SYNC_IF_NO_CHANGES_FOR to 20 seconds for the sake of testing this PR so you can check this for example by adding a new topic in a channel editor and navigating to /channels URL shortly after that.

There is no information on destination URL in beforeunload event (again, to prevent from malicious use), so I am not sure how to solve it. I think there is no need to be worried about it because our default value for SYNC_IF_NO_CHANGES_FOR is 2 seconds so it shouldn't happen very often and also it will be solved for free as soon as we migrate to using one app. Is it okay for now?

[MisRob]

TODO: Remove "Temporarily increase sync interval" commit before merging Done

[codecov]

Codecov Report

Merging #2699 (4a61f9a) into develop (c767412) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## develop #2699 +/- ##
========================================
Coverage 84.94% 84.94% ========================================
Files 301 301 Lines 15227 15227 ========================================
Hits 12934 12934 Misses 2293 2293 

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44518a3...5b5ecab. Read the comment docs.

[rtibbles]

The logout flow seems to be mostly covered, but this might cause aborted sync requests in both the logout and non-logout navigating away from the page flows.

[rtibbles]

Flagging that this won't be translated just yet.

[rtibbles]

Manual testing checks out.

[MisRob]

Description

Inform users about unsaved changes and sync them before they leave the application.

Issue Addressed

Closes#2496

Steps to Test

• Increase SYNC_IF_NO_CHANGES_FOR, for example to 20 seconds, so you're able to leave the application comfortably in all possible ways
• Make some updates that use /sync endpoint, for example add a topic to a channel in the channel editor or update a node in the edit modal
• Leave the app shortly after that by closing a window or logging out

Implementation Notes

This PR adds SyncProgressPlugin Vuex plugin that updates unsavedChanges to true/false based on whether there are some unsaved changes in the IndexedDB table where changes for sync are kept. The value will be updated on each change of the table. The plugin is installed during an app initialization.

Leaving by closing a window

During an app initialization, beforeunload handler is defined. The handler runs server sync immediately and prevents unload if there are any unsavedChanges by showing the default browser prompt to a user who can then decide if they really want to leave while still having some unsaved changes.

Leaving by using logout function

When logging out, a custom confirmation dialog is displayed if there are any unsavedChanges and server sync is triggered immediately. If user confirms that they want to leave, logout will be performed and additionally, logoutConfirmed:true will be saved to sessionStorage. logoutConfirmed value is then checked in beforeunload handler and if set to true, the handler won't continue executing. This is to prevent from showing a confirmation dialog the second time in case a user has already confirmed that they want to leave.

---

The main reason behind this implementation is that beforeunload event handler has some limits to prevent developers from introducing malicious functionality. One of those limitations is that decision logic related to preventing unload needs to be synchronous. Because of that, it doesn't seem to be possible to reuse the solution from SavingIndicator .

Checklist

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?

Design notes

Even though it was possible to specify a custom message in the past in beforeunload event handler, most modern browsers have dropped support. Therefore a default browser message (can vary across browsers) will be displayed when leaving the app by closing a window. Because of that, the message that @jtamiacedesigned is used only in a prompt displayed after trying to log out in a regular way.

Comments

I'd appreciate some thoughts on the following:

Saving indicator

I was thinking about connecting SavingIndicator used in edit modal toSyncProgressPlugin instead of polling IndexedDB though at this point, SavingIndicator is implemented to check only for saving progress of nodes being edited whereas unsavedChanges is simply a Boolean value containing information about whether there are any unsaved changes from any part of the app.

Possible solutions:

(1) Leave it as is
(2) Implement SyncProgressPlugin in a much more granular way to contain information about nodes etc.
(3) Simplify SavingIndicator to check whether there are any unsaved changes by calling areAllChangesSaved getter, not only those related to nodes being currently edited in the edit modal.

I like consistency of (3) and it seems to make sense from UX point of view because with approaches (1) and (2), we could run into a situation when saving indicator would not visible but "Unsaved changes" prompt could be still displayed when trying to exit the app while having some unsaved changes related to nodes that are currently not being edited. Thoughts?

Navigation between apps

Due to our multiple apps architecture, the "Unsaved changes" prompt can be displayed when navigating between apps when there are unsaved changes which disrupts user experience. I increased SYNC_IF_NO_CHANGES_FOR to 20 seconds for the sake of testing this PR so you can check this for example by adding a new topic in a channel editor and navigating to /channels URL shortly after that.

There is no information on destination URL in beforeunload event (again, to prevent from malicious use), so I am not sure how to solve it. I think there is no need to be worried about it because our default value for SYNC_IF_NO_CHANGES_FOR is 2 seconds so it shouldn't happen very often and also it will be solved for free as soon as we migrate to using one app. Is it okay for now?

[MisRob]

TODO: Remove "Temporarily increase sync interval" commit before merging Done

[codecov]

Codecov Report

Merging #2699 (4a61f9a) into develop (c767412) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## develop #2699 +/- ##
========================================
Coverage 84.94% 84.94% ========================================
Files 301 301 Lines 15227 15227 ========================================
Hits 12934 12934 Misses 2293 2293 

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44518a3...5b5ecab. Read the comment docs.

[rtibbles]

The logout flow seems to be mostly covered, but this might cause aborted sync requests in both the logout and non-logout navigating away from the page flows.

[rtibbles]

Flagging that this won't be translated just yet.

[rtibbles]

Manual testing checks out.