Save the last-non-editor-route in redux instead of calculating it

[p-jackson]

When closing the block editor, we take the user back to the last non-editor page they were on. We determine the last non-editor state by searching through the action log.

The problem is that the action log isn't saved to localStorage, so if the user refreshes the page then we lose where they came from and just go to a default location. This bug is made even worse by the navigation sidebar because navigating between items in the sidebar refreshes the page too.

We can't persist the whole action log because that's going to grow unbounded. Instead we'll store just the info we want as a new bit of state in the redux store. Unfortunately that means there's data duplication. 🤞 the redux pattern helps us keep it in sync.

Changes proposed in this Pull Request

• Add lastNonEditorRoute state to redux store
• Persist lastNonEditorRoute state using the withSchemaValidation higer-order-reducer
• Re-write the getLastNonEditorRoute selector so it doesn't depend on the action log
• Update tests

Luckily, block editor navigation is the only user of the getLastNonEditorRoute selector, so we don't have to worry about this change breaking any other features.

Testing instructions

• Checkout branch
• Open Calypso and navigate to the block editor via the pages list
• Refresh the page
• Exit the block editor
• You should be taken back to the pages list (previously you would have gone to customer home, which is the default location when we don't know where in Calypso came from)
• If you have the nav sidebar enabled then:
  • Navigate to the block editor via the pages list
  • Create a new page using the sidebar
  • Navigate between pages using the sidebar
  • Exit the block editor (the link to do so should say "View Pages" or something similar)
  • You should be taken back to the pages list

[matticbot]

Test live: https://calypso.live/?branch=fix/remember-last-calypso-location-in-block-editor

[wp-desktop]

WordPress Desktop CI Failure for job "wp-desktop-source".

@p-jackson please inspect this job's build steps for breaking changes at this link. For temporal failures, you may try to "Rerun Workflow from Failed".

Please also ensure this branch is rebased off latest Calypso.

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~176 bytes added 📈 [gzipped])

name         parsed_size           gzip_size
entry-main        +251 B  (+0.0%)      +87 B  (+0.0%)
entry-login       +251 B  (+0.0%)      +89 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~109 bytes removed 📉 [gzipped])

name              parsed_size           gzip_size
gutenberg-editor       -367 B  (-0.0%)     -109 B  (-0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

wp-desktop ci passing, closing review

[autumnfjeld]

@p-jackson I can confirm the UI tests are working as written in the PR description.

I have a question about some other scenarios.
One scenario:

• Navigate to "Media" e.g. http://calypso.localhost:3000/media/my.blog
• click the Write button in the top right hand corner
• Now in the block-editor
• Click the W logo in the top left to exit the block-editor
• In the side bar there is a link that says "View Posts", so it takes user back to posts , even though they came from Media.

I have no idea if this falls into the issue that this PR solves. Only curious as I was playing around.

[p-jackson]

In the side bar there is a link that says "View Posts", so it takes user back to posts , even though they came from Media.

@autumnfjeld This PR isn't aiming to address that. I'm not sure, but I imagine that's by design. There's a few explicit places the user will be taken back to, not to just any old place in Calypso.

It's a good real-world example of React/Redux integration actually.

<CalypsoifyIframe> is the React component that pulls in the editor and deals with Calypso integration. Its mapStateToProps() function is what's deciding where closing the editor should go back to. And you'll notice it doesn't call the getLastNonEditorRoute() selector (which this PR modifies) directly. Instead it calls the getEditorCloseConfig() selector.

Taking a look at getEditorCloseConfig() you can see it does a few things, including calling getLastNonEditorRoute(). But ultimately it means the (W) button will only take you back to a few places: themes gallery, customer home, page list, post list.

[autumnfjeld]

I'll go ahead a leave my new-automattician-summary here. 🥇

Here is my recap of this PR, that is PR learning experience (combining info from Redux docs, PR, and wp-calypso docs).

Each redux state has a folder in wp-calypso/client/state/ and the root module /state/index.js exports a single reducer function.
The redux store is created with this single reducer function in that index.js. (Side note: Reducers create new State from the previous State and an ACTION instruction)
This PR alters the existing /state/route reducer by adding a sub-reducer, last-non-editor-route, as is evident in the combineReducer function in /state/route/reducer.js
The last-non-editor-route reducer will be responsible for saving the url string of the last non-editor page the user was on.
The lastNonEditorRoute uses withSchemeValidation , which causes the state to be persisted in IndexedDB and causes the reducer to go through json schema validation.
I read through the data-persistence doc, and wanted to summarize to make sure I understand.
I get the broad idea that storing the Redux state in browser storage is desirable for a peppy page load. The initial paragraphs lead me to believe this is done for the whole Redux state tree. Then follows discussion on issues & caveats with subtrees and then describes how serialization overcomes these issues, and instructs to use hasCustomPersistence = true in these problematic subtree situations. (At least that is my takeaway, admittedly I don’t have a deep feel for some of these concepts.)
Then doc goes on to describe another issue, which is data shape changing over time. This makes sense. And then schema validation is discussed as a solution to the evolving data shape issue. Makes sense.
My first takeaway was, that the whole Redux state IS indeed stored in IndexedDB. But sometimes special handling of some parts of the state is needed.
But then the very end of the doc says “Opt-in to Persistence” using withSchemaValidation, which is what you said Phillip. I suppose I find it confusing that the actual ‘how to’ for making a part of the state persistent is at the very end of the doc.