Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add admin menu browser persistence (via schema) #45957

Merged
merged 3 commits into from
Sep 29, 2020

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Sep 28, 2020

This PR addresses #45876 by adding the required schema and HO reducer to enable browser persistence for menus portion of the adminMenu state slice.

This ensures that once the initial API response is cached in the browser IndexDB there is no (less of a) perceivable lag between Calypso UI "load" and the admin menu being "ready" and "available".

Note: this will require that #45939 is merged before we can merge this PR. Now merged 🥳

Changes proposed in this Pull Request

  • Adds a schema.json for the menus portion of the adminMenu state.
  • Wraps the menus portion of the adminMenu state with the schema validation reducer to enable browser peristence of state.

Testing instructions

  • Checkout this PR.
  • Apply blog sticker nav-unification to test site.
  • Run calypso - yarn && ENTRY_LIMIT=entry-main SECTION_LIMIT=home yarn start
  • Visit http://calypso.localhost:3000/?flags=nav-unification.
  • You should see the menu rendered from the static data as per Nav Unification - add scaffolding for admin menu experience from static data #45836
  • Once the API request completes you should see the nav re-rendered using the "correct" data from the API endpoint.
  • Now reload your browser - this time you should not see the nav rendered from the static data but rather rendered semi-instantly from the cached data in the browser storage.
  • Open DevTools. Goto Application tab. Look for Storage in the left hand sidebar. Expand the IndexDB accordion. Find the calypso_store DB entry. Look for the adminMenu slice of state persisted in the DB. Make sure it matches what is stored in state (try state.adminMenu in the Console tab).

Screen Shot 2020-09-28 at 12 24 36

Fixes #45876

@getdave getdave self-assigned this Sep 28, 2020
@matticbot
Copy link
Contributor

@getdave getdave added the [Feature] Calypso & wp-admin Navigation All navigation in Calypso and wp-admin, and the unified transitions between the two. label Sep 28, 2020
@getdave getdave requested a review from a team September 28, 2020 11:18
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 28, 2020
@matticbot
Copy link
Contributor

matticbot commented Sep 28, 2020

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

Async-loaded Components (~137 bytes added 📈 [gzipped])

name                                          parsed_size           gzip_size
async-load-my-sites-sidebar-unified-switcher       +413 B  (+0.2%)     +137 B  (+0.2%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

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.

@getdave getdave marked this pull request as ready for review September 28, 2020 15:02
@getdave getdave force-pushed the add/admin-menu-browser-persistence branch from aac7f52 to d7cf000 Compare September 28, 2020 15:37
@mreishus
Copy link
Contributor

This works properly in my testing. I noticed something strange, but I think it's a limitation of firefox more than anything. In the firefox devtools, if you check the value of the redux-state-1234:adminMenu key, you'll only see the first 10,000 characters of the information. The rest is truncated. Additionally, the "parsed value" shows only a string, because the json is truncated and can't be parsed. However, the feature continues to work correctly, and the indexedDb info looks fine in chrome, so I suspect it's only a visual issue with Firefox's dev tools.

@getdave getdave requested a review from a team September 29, 2020 08:50
Copy link
Contributor

@cpapazoglou cpapazoglou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works as expected! Huge UX improvement! Thanks

@cpapazoglou
Copy link
Contributor

cpapazoglou commented Sep 29, 2020

I re-ran some failing tests

@cpapazoglou cpapazoglou removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 29, 2020
@getdave getdave force-pushed the add/admin-menu-browser-persistence branch from 253741d to e1bab97 Compare September 29, 2020 10:37
@getdave getdave force-pushed the add/admin-menu-browser-persistence branch from e1bab97 to c3b589c Compare September 29, 2020 12:57
@getdave getdave merged commit ebde236 into master Sep 29, 2020
@getdave getdave deleted the add/admin-menu-browser-persistence branch September 29, 2020 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Calypso & wp-admin Navigation All navigation in Calypso and wp-admin, and the unified transitions between the two.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Persist admin menu state to browser storage to ensure quicker render of menu.
5 participants