WordPress.org

Make WordPress Core

Opened 19 months ago

Closed 10 months ago

Last modified 10 months ago

#25439 closed enhancement (fixed)

Prevent loss of unsaved customizer settings or provide means of restoring

Reported by: westonruter Owned by: helen
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: has-patch commit
Focuses: javascript Cc:

Description

This is to revisit #20692, opened by koopersmith for 3.4:

Losing your changes is no fun. ¶ The way I see it, we have two options:

1) Show an AYS when leaving the customizer by using an onbeforeunload handler.
2) Store unsaved customizations in a customize_data_$stylesheet option. The fact that certain customize settings persist across theme changes (options) while others don't (theme_mods and some theme-specific options) could pose a problem.

In the resolution of that ticket, neither of these options were chosen. The beforeunload handler with confirmation dialog (first option) was discouraged, and then another suggestion was provided to just make the Save button more prominent so that users would not neglect to press it before leaving the customizer. However, it seems the more prominent placement of the button is not enough.

For the Widgets UI Refresh feature-as-plugin effort, a user test was put together to test integration with the Customizer. In the video at ~7:30, after the user had made changes to the controls and previewed them, he navigated away from the page. He forgot to hit Save & Publish, or didn't notice it there, and so he lost all of his changes. See the video here: ​http://make.wordpress.org/ui/2013/09/18/widgets-sept-16-chat-notes/#comment-23907

For the 2nd option (storing unsaved settings), another solution than storing drafted settings in a DB option is this: now that autosave will store the drafted post in localStorage, we could do the same for settings in the customizer. Since all of the settings are stored in memory as JS objects, when leaving the customizer with unsaved changes it could store those settings in localStorage.

After having accidentally abandoned changes, when returning to the customizer, the initially-disabled "Saved" button could then swap out with an enabled "Restore" button. As soon as a change is made to a setting on the page, the Restore button would replace with the "Save & Publish" button, and the localStorage-preserved settings would be discarded. But if the user hits "Restore" then the localStorage settings would be populated into the customizer settings and the button would also change out from "Restore" to "Save & Publish".

Attachments (5)

25439.diff (1.2 KB) - added by westonruter 10 months ago.
Prompt user with AYS dialog if leaving the Customizer with unsaved changes. Commit: https://github.com/x-team/wordpress-develop/commit/82349c18f8a0b13a4cefba031d6da61e6205189a PR: https://github.com/x-team/wordpress-develop/pull/23
25439.1.diff (3.6 KB) - added by westonruter 10 months ago.
Relay saved and change events to parent. Keep track of customizer iframe dirty state. Add beforeunload AYS for navigating away from page with customize-loader. Commits: https://github.com/x-team/wordpress-develop/compare/82349c18f8a0b13a4cefba031d6da61e6205189a...f21097b40e32ee03c60f676f3e8d973151cb52e0 Amended PR: https://github.com/x-team/wordpress-develop/pull/23
25439.2.diff (6.5 KB) - added by westonruter 10 months ago.
Changes: Show AYS dialog if customizer dirty and returning to opener page. Cleanup formatting for jshint. New commits: https://github.com/x-team/wordpress-develop/compare/f21097b40e32ee03c60f676f3e8d973151cb52e0...0337bd8537426de554a03807003eec734fe895a6 Amended PR: https://github.com/x-team/wordpress-develop/pull/23
25439.3.diff (9.0 KB) - added by westonruter 10 months ago.
Changes: Skip AYS dialog if user clicked Close/Cancel button. Only show confirmation if using browser navigation. New commit: https://github.com/x-team/wordpress-develop/commit/177de805d407045cb85dc21de1e46c778f674cef Amended PR: https://github.com/x-team/wordpress-develop/pull/23
25439.4.diff (8.6 KB) - added by westonruter 10 months ago.
Remove confirmClose. Added commit: https://github.com/x-team/wordpress-develop/commit/178c5872da30f6d37c30137e61973079558ac178 PR: https://github.com/x-team/wordpress-develop/pull/23

Download all attachments as: .zip

Change History (19)

comment:1 @ircbot10 months ago

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.

comment:2 @helen10 months ago

  • Milestone changed from Awaiting Review to 4.0

Moving to investigate alongside #28655.

comment:3 @helen10 months ago

  • Focuses javascript added
  • Keywords needs-patch added

comment:4 @westonruter10 months ago

  • Keywords has-patch added; needs-patch removed

beforeload AYS confirmation implemented in 25439.diff

comment:5 @ocean9010 months ago

  • Keywords needs-patch added; has-patch removed

As mentioned on IRC, patch doesn't work for theme previews.

@westonruter10 months ago

Relay saved and change events to parent. Keep track of customizer iframe dirty state. Add beforeunload AYS for navigating away from page with customize-loader. Commits: https://github.com/x-team/wordpress-develop/compare/82349c18f8a0b13a4cefba031d6da61e6205189a...f21097b40e32ee03c60f676f3e8d973151cb52e0 Amended PR: https://github.com/x-team/wordpress-develop/pull/23

comment:6 @westonruter10 months ago

In 25439.1.diff accounting for case where user navigates away (e.g. via going too far back in browser history) from themes.php while the customize-loader is open. What is not accounted for yet is preventing the user from going back one step from the theme preview back to the themes page. I have some drafted code not included in this patch, but it is tricky since the popstate event is not cancelable.

@westonruter10 months ago

Changes: Show AYS dialog if customizer dirty and returning to opener page. Cleanup formatting for jshint. New commits: https://github.com/x-team/wordpress-develop/compare/f21097b40e32ee03c60f676f3e8d973151cb52e0...0337bd8537426de554a03807003eec734fe895a6 Amended PR: https://github.com/x-team/wordpress-develop/pull/23

comment:7 @westonruter10 months ago

  • Keywords has-patch added; needs-patch removed

25439.2.diff now handles showing AYS dialog if attempting to leave Customizer with a dirty state to go back to the page which opened the customizer. So the three known cases where an AYS needs to be displayed for the Customizer are now handled.

@westonruter10 months ago

Changes: Skip AYS dialog if user clicked Close/Cancel button. Only show confirmation if using browser navigation. New commit: https://github.com/x-team/wordpress-develop/commit/177de805d407045cb85dc21de1e46c778f674cef Amended PR: https://github.com/x-team/wordpress-develop/pull/23

comment:8 follow-up: @westonruter10 months ago

I also realized that if the user clicks the Cancel button, then they may intentionally be closing the Customizer and shouldn't be prompted with a AYS dialog. This is addressed in 25439.3.diff.

comment:9 in reply to: ↑ 8 ; follow-up: @celloexpressions10 months ago

Replying to westonruter:

I also realized that if the user clicks the Cancel button, then they may intentionally be closing the Customizer and shouldn't be prompted with a AYS dialog. This is addressed in 25439.3.diff.

They're almost certainly intentionally closing it, but if there are unsaved changes, odds are they aren't aware of that. The point of this would be to show the confirmation if they're about to leave the customizer in any way and there are unsaved changes, which I believe 25439.2.diff does.

comment:10 in reply to: ↑ 9 @westonruter10 months ago

Replying to celloexpressions:

They're almost certainly intentionally closing it, but if there are unsaved changes, odds are they aren't aware of that. The point of this would be to show the confirmation if they're about to leave the customizer in any way and there are unsaved changes, which I believe 25439.2.diff does.

Cool. I removed confirmClose but retained some other improvements in 25439.4.diff.

comment:11 @celloexpressions10 months ago

  • Keywords commit added

In #28655, I'm proposing replacing the "back" button terminology with "close" to avoid confusion with .control-panel-back (although looking at your patch I think I missed a bunch of instances). Our patches will conflict there, but we can tweak as needed depending on what goes in first.

Other than that, I've reviewed 25439.4.diff pretty thoroughly and it looks good to go.

comment:12 @ircbot10 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:13 @helen10 months ago

  • Owner set to helen
  • Resolution set to fixed
  • Status changed from new to closed

In 29025:

Prompt the user before leaving the Customizer if they have unsaved changes. props westonruter. fixes #25439.

comment:14 @helen10 months ago

Closing this as fixed with the AYS - if you want to investigate restoration mechanisms, let's do a new ticket (and not 4.0).

Note: See TracTickets for help on using tickets.