From 7aa2b97175cb3dfe5577fe4efe5a07c778206ca5 Mon Sep 17 00:00:00 2001 From: Dave Macaulay Date: Tue, 2 Jun 2020 16:07:51 -0500 Subject: [PATCH 1/8] Update Home Page content after cache hit (#2230) --- packages/venia-ui/lib/RootComponents/CMS/cms.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/venia-ui/lib/RootComponents/CMS/cms.js b/packages/venia-ui/lib/RootComponents/CMS/cms.js index 6545af0241..07b3437e10 100644 --- a/packages/venia-ui/lib/RootComponents/CMS/cms.js +++ b/packages/venia-ui/lib/RootComponents/CMS/cms.js @@ -17,7 +17,8 @@ const CMSPage = props => { variables: { id: Number(id), onServer: false - } + }, + fetchPolicy: 'cache-and-network' }); if (error) { @@ -27,7 +28,7 @@ const CMSPage = props => { return
Page Fetch Error
; } - if (loading) { + if (loading && (!data && !data.cmsPage)) { return fullPageLoadingIndicator; } From 41fe91e002e192be2efa2c94d8039f06f39bb3a2 Mon Sep 17 00:00:00 2001 From: Dave Macaulay Date: Wed, 3 Jun 2020 16:02:08 -0500 Subject: [PATCH 2/8] Update Home Page content after cache hit (#2230) - Remove redundant loading flag and null response on no data --- packages/venia-ui/lib/RootComponents/CMS/cms.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/venia-ui/lib/RootComponents/CMS/cms.js b/packages/venia-ui/lib/RootComponents/CMS/cms.js index 07b3437e10..afbc99b23c 100644 --- a/packages/venia-ui/lib/RootComponents/CMS/cms.js +++ b/packages/venia-ui/lib/RootComponents/CMS/cms.js @@ -13,7 +13,7 @@ import defaultClasses from './cms.css'; const CMSPage = props => { const { id } = props; const classes = mergeClasses(defaultClasses, props.classes); - const { loading, error, data } = useQuery(cmsPageQuery, { + const { error, data } = useQuery(cmsPageQuery, { variables: { id: Number(id), onServer: false @@ -28,12 +28,8 @@ const CMSPage = props => { return
Page Fetch Error
; } - if (loading && (!data && !data.cmsPage)) { - return fullPageLoadingIndicator; - } - if (!data) { - return null; + return fullPageLoadingIndicator; } const { content_heading, title } = data.cmsPage; From 6fdb02abdd617699d600c6fbdb2673d1f2dccfbd Mon Sep 17 00:00:00 2001 From: Dave Macaulay Date: Tue, 9 Jun 2020 16:21:37 -0500 Subject: [PATCH 3/8] PWA-471: Update Home Page content after cache hit (#2230) - Add page loading state to app context - Add loading icon into header - Trigger page loading from CMS root component --- .../lib/store/actions/app/actions.js | 3 ++- .../lib/store/actions/app/asyncActions.js | 3 +++ packages/peregrine/lib/store/reducers/app.js | 9 +++++++- .../peregrine/lib/talons/Header/useHeader.js | 5 ++-- .../venia-ui/lib/RootComponents/CMS/cms.js | 11 ++++++++- .../venia-ui/lib/components/Header/header.css | 3 +++ .../venia-ui/lib/components/Header/header.js | 6 ++++- .../components/PageLoadingIndicator/index.js | 1 + .../PageLoadingIndicator/indicator.css | 17 ++++++++++++++ .../PageLoadingIndicator/indicator.js | 23 +++++++++++++++++++ 10 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 packages/venia-ui/lib/components/PageLoadingIndicator/index.js create mode 100644 packages/venia-ui/lib/components/PageLoadingIndicator/indicator.css create mode 100644 packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js diff --git a/packages/peregrine/lib/store/actions/app/actions.js b/packages/peregrine/lib/store/actions/app/actions.js index 7595d60fad..d062e3db6e 100644 --- a/packages/peregrine/lib/store/actions/app/actions.js +++ b/packages/peregrine/lib/store/actions/app/actions.js @@ -7,7 +7,8 @@ const actionTypes = [ 'SET_OFFLINE', 'TOGGLE_SEARCH', 'EXECUTE_SEARCH', - 'MARK_ERROR_HANDLED' + 'MARK_ERROR_HANDLED', + 'SET_PAGE_LOADING' ]; export default createActions(...actionTypes, { prefix }); diff --git a/packages/peregrine/lib/store/actions/app/asyncActions.js b/packages/peregrine/lib/store/actions/app/asyncActions.js index 15b759d94c..55402579b9 100644 --- a/packages/peregrine/lib/store/actions/app/asyncActions.js +++ b/packages/peregrine/lib/store/actions/app/asyncActions.js @@ -8,3 +8,6 @@ export const closeDrawer = () => async dispatch => export const toggleSearch = () => async dispatch => dispatch(actions.toggleSearch()); + +export const setPageLoading = pageLoading => async dispatch => + dispatch(actions.setPageLoading(pageLoading)); diff --git a/packages/peregrine/lib/store/reducers/app.js b/packages/peregrine/lib/store/reducers/app.js index 1970bcced4..f4a992e2cd 100644 --- a/packages/peregrine/lib/store/reducers/app.js +++ b/packages/peregrine/lib/store/reducers/app.js @@ -10,7 +10,8 @@ const initialState = { isOnline: navigator.onLine, overlay: false, searchOpen: false, - pending: {} + pending: {}, + isPageLoading: false }; const reducerMap = { @@ -39,6 +40,12 @@ const reducerMap = { isOnline: false, hasBeenOffline: true }; + }, + [actions.setPageLoading]: (state, { payload }) => { + return { + ...state, + isPageLoading: payload + }; } }; diff --git a/packages/peregrine/lib/talons/Header/useHeader.js b/packages/peregrine/lib/talons/Header/useHeader.js index 50d3c528c2..22b9aeb57f 100644 --- a/packages/peregrine/lib/talons/Header/useHeader.js +++ b/packages/peregrine/lib/talons/Header/useHeader.js @@ -3,7 +3,7 @@ import { useAppContext } from '@magento/peregrine/lib/context/app'; export const useHeader = () => { const [ - { hasBeenOffline, isOnline, searchOpen }, + { hasBeenOffline, isOnline, searchOpen, isPageLoading }, { toggleSearch } ] = useAppContext(); @@ -15,6 +15,7 @@ export const useHeader = () => { handleSearchTriggerClick, hasBeenOffline, isOnline, - searchOpen + searchOpen, + isPageLoading }; }; diff --git a/packages/venia-ui/lib/RootComponents/CMS/cms.js b/packages/venia-ui/lib/RootComponents/CMS/cms.js index afbc99b23c..ce9b3bee5c 100644 --- a/packages/venia-ui/lib/RootComponents/CMS/cms.js +++ b/packages/venia-ui/lib/RootComponents/CMS/cms.js @@ -9,17 +9,19 @@ import { Meta, Title } from '../../components/Head'; import { mergeClasses } from '../../classify'; import defaultClasses from './cms.css'; +import { useAppContext } from '@magento/peregrine/lib/context/app'; const CMSPage = props => { const { id } = props; const classes = mergeClasses(defaultClasses, props.classes); - const { error, data } = useQuery(cmsPageQuery, { + const { loading, error, data } = useQuery(cmsPageQuery, { variables: { id: Number(id), onServer: false }, fetchPolicy: 'cache-and-network' }); + const [{ isPageLoading }, { setPageLoading }] = useAppContext(); if (error) { if (process.env.NODE_ENV !== 'production') { @@ -32,6 +34,13 @@ const CMSPage = props => { return fullPageLoadingIndicator; } + // Ensure we mark the page as loading while we check the network for updates + if (loading && data && !isPageLoading) { + setPageLoading(true); + } else if (!loading && data && isPageLoading) { + setPageLoading(false); + } + const { content_heading, title } = data.cmsPage; const headingElement = diff --git a/packages/venia-ui/lib/components/Header/header.css b/packages/venia-ui/lib/components/Header/header.css index 5230924ba6..b132d0673a 100755 --- a/packages/venia-ui/lib/components/Header/header.css +++ b/packages/venia-ui/lib/components/Header/header.css @@ -39,6 +39,9 @@ .primaryActions { grid-area: primary; justify-self: start; + display: grid; + grid-template-columns: 1fr 1fr; + align-items: center; } .secondaryActions { diff --git a/packages/venia-ui/lib/components/Header/header.js b/packages/venia-ui/lib/components/Header/header.js index ee49397b4e..d06c62e716 100644 --- a/packages/venia-ui/lib/components/Header/header.js +++ b/packages/venia-ui/lib/components/Header/header.js @@ -12,6 +12,7 @@ import { useHeader } from '@magento/peregrine/lib/talons/Header/useHeader'; import { mergeClasses } from '../../classify'; import defaultClasses from './header.css'; +import PageLoadingIndicator from '../PageLoadingIndicator'; const SearchBar = React.lazy(() => import('../SearchBar')); @@ -20,7 +21,8 @@ const Header = props => { handleSearchTriggerClick, hasBeenOffline, isOnline, - searchOpen + searchOpen, + isPageLoading } = useHeader(); const classes = mergeClasses(defaultClasses, props.classes); @@ -39,12 +41,14 @@ const Header = props => { ) : null; + const pageLoading = isPageLoading ? : null; return (
+ {pageLoading}
{ + const classes = mergeClasses(defaultClasses, props.classes); + + return ( +
+ + {props.children} +
+ ); +}; + +export default PageLoadingIndicator; From bbb792cdd3ca3b63749406e57458ccac1e8cb1bc Mon Sep 17 00:00:00 2001 From: Dave Macaulay Date: Tue, 9 Jun 2020 16:24:40 -0500 Subject: [PATCH 4/8] PWA-471: Update Home Page content after cache hit (#2230) --- .../venia-ui/lib/components/PageLoadingIndicator/indicator.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js b/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js index 7912e55922..9704a95419 100644 --- a/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js +++ b/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js @@ -15,7 +15,6 @@ const PageLoadingIndicator = props => { size={24} classes={{ icon: classes.indicator }} /> - {props.children}
); }; From 33d96dbe21b3251e5c48456961fc99095e843e08 Mon Sep 17 00:00:00 2001 From: Dave Macaulay Date: Wed, 10 Jun 2020 15:26:48 -0500 Subject: [PATCH 5/8] PWA-471: Update Home Page content after cache hit (#2230) - Add test coverage for new isPageLoading in talon & header --- .../talons/Header/__tests__/useHeader.spec.js | 72 +++++++++++++++++++ .../__snapshots__/header.spec.js.snap | 30 ++++++++ .../Header/__tests__/header.spec.js | 52 ++++++++++++++ 3 files changed, 154 insertions(+) create mode 100644 packages/peregrine/lib/talons/Header/__tests__/useHeader.spec.js create mode 100644 packages/venia-ui/lib/components/Header/__tests__/__snapshots__/header.spec.js.snap create mode 100644 packages/venia-ui/lib/components/Header/__tests__/header.spec.js diff --git a/packages/peregrine/lib/talons/Header/__tests__/useHeader.spec.js b/packages/peregrine/lib/talons/Header/__tests__/useHeader.spec.js new file mode 100644 index 0000000000..d2ba370674 --- /dev/null +++ b/packages/peregrine/lib/talons/Header/__tests__/useHeader.spec.js @@ -0,0 +1,72 @@ +import React, { useEffect } from 'react'; +import { useAppContext } from '@magento/peregrine/lib/context/app'; +import { useHeader } from '../useHeader'; +import { createTestInstance } from '../../../index'; +import { act } from 'react-test-renderer'; + +jest.mock('@magento/peregrine/lib/context/app', () => { + const api = {}; + const state = {}; + return { + useAppContext: jest.fn(() => [state, api]) + }; +}); + +const log = jest.fn(); +const Component = () => { + const talonProps = useHeader(); + + useEffect(() => { + log(talonProps); + }, [talonProps]); + + return
; +}; + +test('useHeader returns correct values from useAppContext', () => { + useAppContext.mockImplementation(() => { + return [ + { + hasBeenOffline: false, + isOnline: true, + searchOpen: false, + isPageLoading: false + }, + { + toggleSearch: jest.fn() + } + ]; + }); + + createTestInstance(); + + expect(log).toHaveBeenCalledWith({ + handleSearchTriggerClick: expect.any(Function), + hasBeenOffline: false, + isOnline: true, + isPageLoading: false, + searchOpen: false + }); +}); + +test('handleSearchTriggerClick calls toggleSearch from useAppContext', () => { + const toggleSearchFn = jest.fn(); + + useAppContext.mockImplementation(() => { + return [ + {}, + { + toggleSearch: toggleSearchFn + } + ]; + }); + + const component = createTestInstance(); + const talonProps = component.root.findByProps({ id: 'header' }).props; + + act(() => { + talonProps.handleSearchTriggerClick(); + }); + + expect(toggleSearchFn).toHaveBeenCalled(); +}); diff --git a/packages/venia-ui/lib/components/Header/__tests__/__snapshots__/header.spec.js.snap b/packages/venia-ui/lib/components/Header/__tests__/__snapshots__/header.spec.js.snap new file mode 100644 index 0000000000..799935f6e1 --- /dev/null +++ b/packages/venia-ui/lib/components/Header/__tests__/__snapshots__/header.spec.js.snap @@ -0,0 +1,30 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`verify Header can render in default state 1`] = ` +
+
+
+ +
+ +
+ + +
+
+
+`; diff --git a/packages/venia-ui/lib/components/Header/__tests__/header.spec.js b/packages/venia-ui/lib/components/Header/__tests__/header.spec.js new file mode 100644 index 0000000000..3844950a0b --- /dev/null +++ b/packages/venia-ui/lib/components/Header/__tests__/header.spec.js @@ -0,0 +1,52 @@ +import React from 'react'; +import { createTestInstance } from '@magento/peregrine'; +import Header from "../header"; +import { useHeader } from '@magento/peregrine/lib/talons/Header/useHeader'; + +jest.mock('../../../classify'); +jest.mock('../../Logo', () => 'Logo'); +jest.mock('../cartTrigger', () => 'CartTrigger'); +jest.mock('../navTrigger', () => 'NavTrigger'); +jest.mock('../searchTrigger', () => 'SearchTrigger'); +jest.mock('../onlineIndicator', () => 'OnlineIndicator'); +jest.mock('../../PageLoadingIndicator', () => () =>
); + +jest.mock('@magento/venia-drivers', () => ({ + resourceUrl: jest.fn(url => url), + Link: jest.fn(() => null), + Route: jest.fn(() => null) +})); + +jest.mock('@magento/peregrine/lib/talons/Header/useHeader', () => { + const state = { + handleSearchTriggerClick: jest.fn(), + hasBeenOffline: false, + isOnline: true, + searchOpen: false, + isPageLoading: false + }; + return { + useHeader: jest.fn(() => state) + }; +}); + +test('verify Header can render in default state', () => { + const component = createTestInstance(
); + + expect(component.toJSON()).toMatchSnapshot(); +}); + +test('verify PageLoadingIndicator is displayed when page is loading', () => { + useHeader.mockImplementation(() => { + return { + handleSearchTriggerClick: jest.fn(), + hasBeenOffline: false, + isOnline: true, + searchOpen: false, + isPageLoading: true + } + }); + + const component = createTestInstance(
); + component.root.findByProps({ id: 'pageLoadingIndicator' }); +}); From 3c999d1f1db963a0c50e3a54341e369449474eff Mon Sep 17 00:00:00 2001 From: Dave Macaulay Date: Wed, 10 Jun 2020 15:27:43 -0500 Subject: [PATCH 6/8] PWA-471: Update Home Page content after cache hit (#2230) --- .../lib/components/Header/__tests__/header.spec.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/venia-ui/lib/components/Header/__tests__/header.spec.js b/packages/venia-ui/lib/components/Header/__tests__/header.spec.js index 3844950a0b..a24b65e1b0 100644 --- a/packages/venia-ui/lib/components/Header/__tests__/header.spec.js +++ b/packages/venia-ui/lib/components/Header/__tests__/header.spec.js @@ -1,6 +1,6 @@ import React from 'react'; import { createTestInstance } from '@magento/peregrine'; -import Header from "../header"; +import Header from '../header'; import { useHeader } from '@magento/peregrine/lib/talons/Header/useHeader'; jest.mock('../../../classify'); @@ -9,7 +9,9 @@ jest.mock('../cartTrigger', () => 'CartTrigger'); jest.mock('../navTrigger', () => 'NavTrigger'); jest.mock('../searchTrigger', () => 'SearchTrigger'); jest.mock('../onlineIndicator', () => 'OnlineIndicator'); -jest.mock('../../PageLoadingIndicator', () => () =>
); +jest.mock('../../PageLoadingIndicator', () => () => ( +
+)); jest.mock('@magento/venia-drivers', () => ({ resourceUrl: jest.fn(url => url), @@ -44,7 +46,7 @@ test('verify PageLoadingIndicator is displayed when page is loading', () => { isOnline: true, searchOpen: false, isPageLoading: true - } + }; }); const component = createTestInstance(
); From f2f6c647d13c5592b9a472499a5cae16a178620d Mon Sep 17 00:00:00 2001 From: Dave Macaulay Date: Thu, 11 Jun 2020 11:06:20 -0500 Subject: [PATCH 7/8] PWA-471: Update Home Page content after cache hit (#2230) --- .../peregrine/lib/store/actions/app/asyncActions.js | 3 --- packages/peregrine/lib/store/reducers/app.js | 2 +- packages/peregrine/lib/talons/Header/useHeader.js | 2 +- packages/venia-ui/lib/RootComponents/CMS/cms.js | 11 ++++++++--- packages/venia-ui/lib/components/Header/header.css | 2 +- packages/venia-ui/lib/components/Header/header.js | 6 ++++-- .../lib/components/PageLoadingIndicator/indicator.css | 2 +- 7 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/peregrine/lib/store/actions/app/asyncActions.js b/packages/peregrine/lib/store/actions/app/asyncActions.js index 55402579b9..15b759d94c 100644 --- a/packages/peregrine/lib/store/actions/app/asyncActions.js +++ b/packages/peregrine/lib/store/actions/app/asyncActions.js @@ -8,6 +8,3 @@ export const closeDrawer = () => async dispatch => export const toggleSearch = () => async dispatch => dispatch(actions.toggleSearch()); - -export const setPageLoading = pageLoading => async dispatch => - dispatch(actions.setPageLoading(pageLoading)); diff --git a/packages/peregrine/lib/store/reducers/app.js b/packages/peregrine/lib/store/reducers/app.js index f4a992e2cd..199b32a703 100644 --- a/packages/peregrine/lib/store/reducers/app.js +++ b/packages/peregrine/lib/store/reducers/app.js @@ -44,7 +44,7 @@ const reducerMap = { [actions.setPageLoading]: (state, { payload }) => { return { ...state, - isPageLoading: payload + isPageLoading: !!payload }; } }; diff --git a/packages/peregrine/lib/talons/Header/useHeader.js b/packages/peregrine/lib/talons/Header/useHeader.js index 22b9aeb57f..a640740185 100644 --- a/packages/peregrine/lib/talons/Header/useHeader.js +++ b/packages/peregrine/lib/talons/Header/useHeader.js @@ -3,7 +3,7 @@ import { useAppContext } from '@magento/peregrine/lib/context/app'; export const useHeader = () => { const [ - { hasBeenOffline, isOnline, searchOpen, isPageLoading }, + { hasBeenOffline, isOnline, isPageLoading, searchOpen }, { toggleSearch } ] = useAppContext(); diff --git a/packages/venia-ui/lib/RootComponents/CMS/cms.js b/packages/venia-ui/lib/RootComponents/CMS/cms.js index ce9b3bee5c..e9e377fdfa 100644 --- a/packages/venia-ui/lib/RootComponents/CMS/cms.js +++ b/packages/venia-ui/lib/RootComponents/CMS/cms.js @@ -21,7 +21,12 @@ const CMSPage = props => { }, fetchPolicy: 'cache-and-network' }); - const [{ isPageLoading }, { setPageLoading }] = useAppContext(); + const [ + { isPageLoading }, + { + actions: { setPageLoading } + } + ] = useAppContext(); if (error) { if (process.env.NODE_ENV !== 'production') { @@ -35,9 +40,9 @@ const CMSPage = props => { } // Ensure we mark the page as loading while we check the network for updates - if (loading && data && !isPageLoading) { + if (loading && !isPageLoading) { setPageLoading(true); - } else if (!loading && data && isPageLoading) { + } else if (!loading && isPageLoading) { setPageLoading(false); } diff --git a/packages/venia-ui/lib/components/Header/header.css b/packages/venia-ui/lib/components/Header/header.css index b132d0673a..06e3af5ac1 100755 --- a/packages/venia-ui/lib/components/Header/header.css +++ b/packages/venia-ui/lib/components/Header/header.css @@ -40,7 +40,7 @@ grid-area: primary; justify-self: start; display: grid; - grid-template-columns: 1fr 1fr; + grid-auto-flow: column; align-items: center; } diff --git a/packages/venia-ui/lib/components/Header/header.js b/packages/venia-ui/lib/components/Header/header.js index d06c62e716..d624f227cc 100644 --- a/packages/venia-ui/lib/components/Header/header.js +++ b/packages/venia-ui/lib/components/Header/header.js @@ -41,14 +41,16 @@ const Header = props => { ) : null; - const pageLoading = isPageLoading ? : null; + const pageLoadingIndicator = isPageLoading ? ( + + ) : null; return (
- {pageLoading} + {pageLoadingIndicator}
Date: Thu, 11 Jun 2020 13:26:55 -0500 Subject: [PATCH 8/8] PWA-471: Update Home Page content after cache hit (#2230) --- .../venia-ui/lib/components/PageLoadingIndicator/indicator.css | 2 +- .../venia-ui/lib/components/PageLoadingIndicator/indicator.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.css b/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.css index 275b444b2d..980f922dcb 100644 --- a/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.css +++ b/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.css @@ -4,7 +4,7 @@ } .indicator { - color: rgb(var(--venia-text-hint)); + --stroke: rgb(var(--venia-text-hint)); } @keyframes spin { diff --git a/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js b/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js index 9704a95419..41c19c1090 100644 --- a/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js +++ b/packages/venia-ui/lib/components/PageLoadingIndicator/indicator.js @@ -13,7 +13,7 @@ const PageLoadingIndicator = props => {
);