-
Notifications
You must be signed in to change notification settings - Fork 2k
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 a First View introduction overlay to the Stats section #6717
Conversation
This a temporary stop-gap until `Main` component and all usage of it is Reduxified.
Enable in development only.
Okay, small update... the "half-loaded" state issue that @shaunandrews found only happens in Safari Private Mode. I believe this is because IndexedDB and localStorage are not available when in private mode. |
Lots of mobile tweaks.
|
||
export function switchedFromDifferentSection( state ) { | ||
const section = state.ui.section; | ||
const routeSets = filter( getActionLog( state ), entry => entry.type === 'ROUTE_SET' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be using the proper ROUTE_SET
constant from action-types
, rather than the string representation.
While we're here: if lodash's filter
is only used in its filter( collection, predicate )
form, then it could be replaced with the native implementation: getActionLog( state ).filter( entry => … )
. An other usage would be the convenient: filter( getActionLog( state ), { type: ROUTE_SET } )
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, fixed!
Nothing jumps out. This PR merely consumes |
let isAbleToAccessLocalStorage = true; | ||
|
||
try { | ||
localStorage.checkIfFirstViewPreferencesCanBeCached = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
localStorage
is on the window
object, so this would break SSR constraints. Do we actually want to test for localStorage here? We use localForage
, I wonder if there's an established way to check elsewhere in Calypso.
Also, putting this in the selector worries me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ehg We will pull this back out of the PR and tackle it in a separate PR where we can discuss more and come to the best solution, taking into account things like SSR. Thanks for pointing this out and questioning it!
Marking this as Ready to Merge as per feedback (thumbs-up) from reviewers above. Thank you for your contribution! |
This PR adds a First View introduction overlay to the Stats section.
The First View introduction overlay is shown to users when they first visit the Stats section.
Clicking the "Got it!" button will hide the First View and show the Stats section.
If the "Don't show this again" checkbox is checked, the First View will never been shown to the user again. If it is not checked, the First View will be shown whenever the user visits the Stats section again.
If the "Don't show this again" checkbox was checked, you can delete the
calypso_store
IndexedDB table in order to reset the Redux state and allow the First View to be shown again.Testing notes:
Support for other sections will be possible once #6651 is completed (Reduxifying
<Main>
).Test live: https://calypso.live/?branch=add/stats-first-view