Data: Migrate post editor persistence with fullscreenMode false #21082
Conversation
aduth
added
[Type] Bug
|
Size Change: +54 B (0%) Total Size: 857 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
LGTM 👍
The settings seem to be stored correctly.
I verified without anything in the local storage the default mode is still full-screen mode.
I opened the editor in a version where fullscreen mode was not yet enabled by default, I created a post in that branch. I switched to this branch and I verified fullscreen mode was not active when the post loaded.
jorgefilipecosta
deleted the
update/migrate-fullscreen-persisted-default
branch
jorgefilipecosta
added
the
Backport to WP 6.7 Beta/RC
We should update the comment that says "Remove this when WordPress 5.5 is released". More broadly, I'm thinking we should probably admit that this isn't a temporary function and come up with a proper API for migrating local settings, since the need for migrating settings has arisen three times now 🙂 |
I agree on both accounts. I initially prompted @youknowriad to include an "expiration date" when this was introduced largely in mind of the fact that maintaining (and requiring the download of) client-side code in perpetuity is far from ideal. Whether we decide to still do these expirations on a case-by-case basis is still up for debate, but as much as we should want to try to avoid a migration by designing up-front in mind for the future, it seems inevitable we'll encounter scenarios like this again. A few miscellaneous observations:
|
jorgefilipecosta
removed
the
Backport to WP 6.7 Beta/RC
Fixes #20956
This pull request seeks to resolve an issue where the fullscreen mode would not default to
falsefor a user who had previously interacted with the editor, but who had not made an explicit choice to enable or disable the fullscreen mode setting.This issue stems in-part from the fact that prior to #20611, there was no default value assigned for the
fullscreenModepreference, and it was implicitly treated asfalse. Thus, as of #20611, even if there was persisted state for the editor, the absence of this default value would cause the new default (true) to take effect.This is compounded with the fact that state is only persisted at the point that any of the preferences within a given store have been changed. Thus, if a user did not change any of the post editor default preferences, the full set of new defaults would be used.
The proposed changes seek to resolve this by considering at first load whether any persisted state had existed previously (for all stores) to use as an indication that the user had previously interacted with the editor. If state had existed, but the user did not have a default value assigned for
fullscreenMode, then a value offalseis assigned.This works in large part due to the fact that the "New User Experience" tips are a preference value. At the point a user dismisses these tips (even if by navigating elsewhere in the admin), this preferenece is persisted and leveraged as a marker for prior interaction.
true.false.true.true.Implementation Notes:
For the purposes of setting a default persistence value, the implementation is based in part on prior art from NUX to welcome guide migration.
While this function was originally marked for removal, given the recent additions of Welcome Mode migration, this is not likely for the near future.
In initial implementations, I had observed some sporadic behavior in whether the default would take effect. I believe this was caused by conflicts with the two separate migrations attempting to merge to the same state. Even changing this mutation was not enough. It may be that since persistence operates with localStorage, there was a race condition causing only one of the two migrations to "take hold". The proposed implementation seeks to try to make this more durable by calling
persistence.setat most one time for both of these migrations.Testing Instructions:
Run through each of the scenarios above, confirming the expected outcomes.
In testing, you will likely want to be running a site on WordPress 5.3.2. For each scenario, start fresh with localStorage by either running
localStorage.clear()in your browser console prior (ideally on a screen other than the editor). For simulating the case of a user having made some prior interaction with the editor, you should perform this action while the Gutenberg plugin is disabled.