From 8a610f37f6c2c025aab9ff41db679dd873c09138 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Tue, 18 Jan 2022 17:19:41 +0800 Subject: [PATCH 1/8] Move full-container/index.scss to primary css folder --- js/src/components/full-container/index.js | 1 - js/src/css/index.scss | 3 +++ .../index.scss => css/shared/_woocommerce-admin.scss} | 0 3 files changed, 3 insertions(+), 1 deletion(-) rename js/src/{components/full-container/index.scss => css/shared/_woocommerce-admin.scss} (100%) diff --git a/js/src/components/full-container/index.js b/js/src/components/full-container/index.js index 63ad4ebf4b..4ac141da6d 100644 --- a/js/src/components/full-container/index.js +++ b/js/src/components/full-container/index.js @@ -6,7 +6,6 @@ import { useEffect } from '@wordpress/element'; /** * Internal dependencies */ -import './index.scss'; import useWPBodyMarginOffsetEffect from './useWPBodyMarginOffsetEffect'; /** diff --git a/js/src/css/index.scss b/js/src/css/index.scss index e87d626d52..517d90b820 100644 --- a/js/src/css/index.scss +++ b/js/src/css/index.scss @@ -4,3 +4,6 @@ // Import Gutenberg components that aren't already imported in the lowest WC Admin version we support @import "./shared/gutenberg-components.scss"; + +// Import GLA-wise styling that based on woocommerce-admin styles. +@import "./shared/woocommerce-admin.scss"; diff --git a/js/src/components/full-container/index.scss b/js/src/css/shared/_woocommerce-admin.scss similarity index 100% rename from js/src/components/full-container/index.scss rename to js/src/css/shared/_woocommerce-admin.scss From b9dd0d0a357f2fadf46ed511d24bf49a7f035da0 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Tue, 18 Jan 2022 17:23:00 +0800 Subject: [PATCH 2/8] Move full-screen/index.scss to primary css folder --- js/src/components/full-screen/index.js | 5 ----- .../index.scss => css/shared/_woocommerce-admin.scss} | 0 2 files changed, 5 deletions(-) rename js/src/{components/full-screen/index.scss => css/shared/_woocommerce-admin.scss} (100%) diff --git a/js/src/components/full-screen/index.js b/js/src/components/full-screen/index.js index c16102db3b..7b596579bd 100644 --- a/js/src/components/full-screen/index.js +++ b/js/src/components/full-screen/index.js @@ -3,11 +3,6 @@ */ import { useEffect } from '@wordpress/element'; -/** - * Internal dependencies - */ -import './index.scss'; - const cssClassNames = { isWPToolbarDisabled: 'is-wp-toolbar-disabled', woocommerceAdminFullScreen: 'woocommerce-admin-full-screen', diff --git a/js/src/components/full-screen/index.scss b/js/src/css/shared/_woocommerce-admin.scss similarity index 100% rename from js/src/components/full-screen/index.scss rename to js/src/css/shared/_woocommerce-admin.scss From 89cf9aa6d8434cef64ef30146a5b0c3970510ea6 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Tue, 18 Jan 2022 18:14:36 +0800 Subject: [PATCH 3/8] Implement useLayout hook to apply full page/content layout --- js/src/css/shared/_woocommerce-admin.scss | 6 ++-- js/src/hooks/useLayout.js | 41 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 js/src/hooks/useLayout.js diff --git a/js/src/css/shared/_woocommerce-admin.scss b/js/src/css/shared/_woocommerce-admin.scss index 77d7bb71ed..b4bb18c1ca 100644 --- a/js/src/css/shared/_woocommerce-admin.scss +++ b/js/src/css/shared/_woocommerce-admin.scss @@ -1,4 +1,5 @@ -.gla-full-container { +// Used in .~/hooks/useLayout.js +.gla-full-content { .woocommerce-layout { padding-top: 0; @@ -51,7 +52,8 @@ } } -.app-full-screen { +// Used in .~/hooks/useLayout.js +.gla-full-page { // hack to fix the margin-top when WC Navigation is not enabled // and width is between 600px and 782px. // without this, the margin-top would be -32px, diff --git a/js/src/hooks/useLayout.js b/js/src/hooks/useLayout.js new file mode 100644 index 0000000000..9487965145 --- /dev/null +++ b/js/src/hooks/useLayout.js @@ -0,0 +1,41 @@ +/** + * External dependencies + */ +import { useEffect } from '@wordpress/element'; + +// gla-* styles are defined in .~/css/shared/_woocommerce-admin.scss +const classNameDict = { + 'full-page': [ + 'woocommerce-admin-full-screen', + 'is-wp-toolbar-disabled', + 'gla-full-page', + ], + + 'full-content': [ 'gla-full-content' ], +}; + +/** + * A hook to attach specified layout styles onto topper DOM nodes when mounting, + * and unattach when unmounting. + * + * @param {'full-page'|'full-content'} layoutName Indicates which layout to be applied. + * - full-page: Display full page layout by hiding top bar, left sidebar and header. + * - full-content: Display full content layout by hiding header and StoreAlerts. + */ +export default function useLayout( layoutName ) { + useEffect( () => { + if ( ! classNameDict.hasOwnProperty( layoutName ) ) { + return; + } + + const bodyClassList = document.body.classList; + const classNames = classNameDict[ layoutName ].filter( + ( name ) => ! bodyClassList.contains( name ) + ); + + bodyClassList.add( ...classNames ); + return () => { + bodyClassList.remove( ...classNames ); + }; + }, [ layoutName ] ); +} From 46c90ee32af18b0bdaef08edf8eebe8178cd579b Mon Sep 17 00:00:00 2001 From: Eason Su Date: Tue, 18 Jan 2022 18:16:35 +0800 Subject: [PATCH 4/8] Use CSS approach to override `margin-top` of #wpbody instead of useWPBodyMarginOffsetEffect hook --- js/src/css/shared/_woocommerce-admin.scss | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/js/src/css/shared/_woocommerce-admin.scss b/js/src/css/shared/_woocommerce-admin.scss index b4bb18c1ca..ca9920dc02 100644 --- a/js/src/css/shared/_woocommerce-admin.scss +++ b/js/src/css/shared/_woocommerce-admin.scss @@ -1,5 +1,12 @@ // Used in .~/hooks/useLayout.js .gla-full-content { + // #wpbody `margin-top` style is set onto DOM node directly in WC Admin. + // Here force override it to 0. + // Ref: https://github.com/woocommerce/woocommerce-admin/blob/95c487247416ab34eb8e492b984e2b068618e0d3/client/header/index.js#L92-L118. + #wpbody { + margin-top: 0 !important; + } + .woocommerce-layout { padding-top: 0; From a5e8509020200139384ed11f48302db2d87a1e5a Mon Sep 17 00:00:00 2001 From: Eason Su Date: Tue, 18 Jan 2022 18:20:27 +0800 Subject: [PATCH 5/8] Move the CSS for hidden Spinner for Suspended StoreAlert to global scope --- js/src/css/shared/_woocommerce-admin.scss | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/js/src/css/shared/_woocommerce-admin.scss b/js/src/css/shared/_woocommerce-admin.scss index ca9920dc02..33549ce761 100644 --- a/js/src/css/shared/_woocommerce-admin.scss +++ b/js/src/css/shared/_woocommerce-admin.scss @@ -1,3 +1,17 @@ +/** + * Workaround to hide the initial loading spinner that causes whole page flickering. + * The flickering issue has been fixed and released with WC-admin 2.9.0. + * - https://github.com/woocommerce/woocommerce-admin/pull/7886 + * - https://github.com/woocommerce/woocommerce-admin/blob/v2.9.0/changelog.txt#L16 + * + * This workaround can be removed after the L-2 version ≥ WC 6.0, which uses WC-admin 2.9.4 + */ +.woocommerce-layout__primary { + > .woocommerce-spinner:first-child { + display: none; + } +} + // Used in .~/hooks/useLayout.js .gla-full-content { // #wpbody `margin-top` style is set onto DOM node directly in WC Admin. @@ -33,11 +47,6 @@ .woocommerce-layout__primary { margin: 0; - // Hide the Spinner for Suspended StoreAlerts. - > .woocommerce-spinner { - display: none; - } - .woocommerce-layout__main { padding: 0; From 28a80522d40eebef6276b2c840ba24eb6d01f985 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Tue, 18 Jan 2022 18:24:20 +0800 Subject: [PATCH 6/8] Replace all FullContainer usages with useLayout --- js/src/components/full-container/index.js | 43 ----------------- .../useWPBodyMarginOffsetEffect.js | 47 ------------------- js/src/edit-free-campaign/index.js | 8 ++-- .../pages/create-paid-ads-campaign/index.js | 8 ++-- js/src/pages/edit-paid-ads-campaign/index.js | 16 ++++--- js/src/settings/edit-phone-number.js | 7 +-- js/src/settings/edit-store-address.js | 8 ++-- 7 files changed, 28 insertions(+), 109 deletions(-) delete mode 100644 js/src/components/full-container/index.js delete mode 100644 js/src/components/full-container/useWPBodyMarginOffsetEffect.js diff --git a/js/src/components/full-container/index.js b/js/src/components/full-container/index.js deleted file mode 100644 index 4ac141da6d..0000000000 --- a/js/src/components/full-container/index.js +++ /dev/null @@ -1,43 +0,0 @@ -/** - * External dependencies - */ -import { useEffect } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import useWPBodyMarginOffsetEffect from './useWPBodyMarginOffsetEffect'; - -/** - * Make the wrapped component display in full container. - * It workarounds WooCommerce-admin's navigation Container component, to display the child components on full pane, without Header and StoreAlerts. - * Actually it does not wrap children elements, but forcefully change WooCommerce-admin layout, to make `.woocommerce-layout__main` occupy the full pane. - * - * ## Usage - * - * ```jsx - * - * - * - * ``` - * - * @see FullScreen - * - * @param {Object} props - * @param {Array} props.children Array of react elements to be rendered. - */ -export default function FullContainer( props ) { - const { children } = props; - - useEffect( () => { - document.body.classList.add( 'gla-full-container' ); - - return () => { - document.body.classList.remove( 'gla-full-container' ); - }; - }, [] ); - - useWPBodyMarginOffsetEffect(); - - return children; -} diff --git a/js/src/components/full-container/useWPBodyMarginOffsetEffect.js b/js/src/components/full-container/useWPBodyMarginOffsetEffect.js deleted file mode 100644 index 94f6270f28..0000000000 --- a/js/src/components/full-container/useWPBodyMarginOffsetEffect.js +++ /dev/null @@ -1,47 +0,0 @@ -/** - * External dependencies - */ -import { useEffect } from '@wordpress/element'; - -/** - * Timeout in milliseconds to check the #wpbody margin-top. - * - * In https://github.com/woocommerce/woocommerce-admin/blob/95c487247416ab34eb8e492b984e2b068618e0d3/client/header/index.js#L92-L118, the timeout used is `200`. - * We use `210` here to run after the WC-Admin code, to be a little bit safer. - */ -const timeoutInMS = 210; - -/** - * Offset the #wpbody margin-top by applying negative margin-top to its child #wpbody-content. - * - * #wpbody margin-top is set after a 200ms timeout in WC Admin. To counter that, we take a similar timeout approach here too. - * - * The code here is based on the code in https://github.com/woocommerce/woocommerce-admin/blob/95c487247416ab34eb8e492b984e2b068618e0d3/client/header/index.js#L92-L118. - */ -const useWPBodyMarginOffsetEffect = () => { - useEffect( () => { - const wpBodyContent = document.querySelector( '#wpbody-content' ); - if ( ! wpBodyContent ) { - return; - } - - const timeoutId = setTimeout( () => { - const wpBody = document.querySelector( '#wpbody' ); - const marginTop = - wpBody && wpBody.style.marginTop - ? `-${ wpBody.style.marginTop }` - : null; - - wpBodyContent.style.marginTop = marginTop; - }, timeoutInMS ); - - return () => { - clearTimeout( timeoutId ); - if ( wpBodyContent ) { - wpBodyContent.style.marginTop = null; - } - }; - }, [] ); -}; - -export default useWPBodyMarginOffsetEffect; diff --git a/js/src/edit-free-campaign/index.js b/js/src/edit-free-campaign/index.js index 725d79c59c..54af06dcc0 100644 --- a/js/src/edit-free-campaign/index.js +++ b/js/src/edit-free-campaign/index.js @@ -12,13 +12,13 @@ import { isEqual } from 'lodash'; * Internal dependencies */ import { useAppDispatch } from '.~/data'; -import FullContainer from '.~/components/full-container'; import TopBar from '.~/components/stepper/top-bar'; import ChooseAudience from '.~/components/free-listings/choose-audience'; import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes'; import useSettings from '.~/components/free-listings/configure-product-listings/useSettings'; import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; import SetupFreeListings from './setup-free-listings'; +import useLayout from '.~/hooks/useLayout'; import useNavigateAwayPromptEffect from '.~/hooks/useNavigateAwayPromptEffect'; import useShippingRates from '.~/hooks/useShippingRates'; import useShippingTimes from '.~/hooks/useShippingTimes'; @@ -83,6 +83,8 @@ function saveShippingData( batchUpsertAction, newData, getGroupKey ) { * The displayed step is driven by `pageStep` URL parameter, to make it easier to permalink and navigate back and forth. */ export default function EditFreeCampaign() { + useLayout( 'full-content' ); + const { targetAudience: savedTargetAudience, getFinalCountries, @@ -220,7 +222,7 @@ export default function EditFreeCampaign() { }; // TODO: Wse ChooseAudience and SetupFreeListings customized for this page. return ( - + <> - + ); } diff --git a/js/src/pages/create-paid-ads-campaign/index.js b/js/src/pages/create-paid-ads-campaign/index.js index 4381fb2944..9cc3ba53da 100644 --- a/js/src/pages/create-paid-ads-campaign/index.js +++ b/js/src/pages/create-paid-ads-campaign/index.js @@ -7,7 +7,7 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import FullContainer from '.~/components/full-container'; +import useLayout from '.~/hooks/useLayout'; import TopBar from '.~/components/stepper/top-bar'; import HelpIconButton from '.~/components/help-icon-button'; import CreatePaidAdsCampaignForm from './create-paid-ads-campaign-form'; @@ -15,8 +15,10 @@ import CreatePaidAdsCampaignForm from './create-paid-ads-campaign-form'; const dashboardURL = getNewPath( {}, '/google/dashboard', {} ); const CreatePaidAdsCampaign = () => { + useLayout( 'full-content' ); + return ( - + <> { backHref={ dashboardURL } /> - + ); }; diff --git a/js/src/pages/edit-paid-ads-campaign/index.js b/js/src/pages/edit-paid-ads-campaign/index.js index 1a3b0a1ab8..96e5546ad3 100644 --- a/js/src/pages/edit-paid-ads-campaign/index.js +++ b/js/src/pages/edit-paid-ads-campaign/index.js @@ -7,8 +7,8 @@ import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ -import FullContainer from '.~/components/full-container'; import TopBar from '.~/components/stepper/top-bar'; +import useLayout from '.~/hooks/useLayout'; import useApiFetchEffect from '.~/hooks/useApiFetchEffect'; import AppSpinner from '.~/components/app-spinner'; import EditPaidAdsCampaignForm from './edit-paid-ads-campaign-form'; @@ -18,6 +18,8 @@ const dashboardURL = getNewPath( {}, '/google/dashboard', {} ); const helpButton = ; const EditPaidAdsCampaign = () => { + useLayout( 'full-content' ); + const { programId } = getQuery(); const { loading, error, data: campaignData } = useApiFetchEffect( { path: `/wc/gla/ads/campaigns/${ programId }`, @@ -25,20 +27,20 @@ const EditPaidAdsCampaign = () => { if ( loading ) { return ( - + <> - + ); } if ( error ) { return ( - + <> { 'google-listings-and-ads' ) } - + ); } return ( - + <> { backHref={ dashboardURL } /> - + ); }; diff --git a/js/src/settings/edit-phone-number.js b/js/src/settings/edit-phone-number.js index a7fca6b7c1..a9d33dc8d8 100644 --- a/js/src/settings/edit-phone-number.js +++ b/js/src/settings/edit-phone-number.js @@ -7,9 +7,9 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import { getSettingsUrl } from '.~/utils/urls'; -import FullContainer from '.~/components/full-container'; import TopBar from '.~/components/stepper/top-bar'; import HelpIconButton from '.~/components/help-icon-button'; +import useLayout from '.~/hooks/useLayout'; import useGoogleMCPhoneNumber from '.~/hooks/useGoogleMCPhoneNumber'; import Section from '.~/wcdl/section'; import AppDocumentationLink from '.~/components/app-documentation-link'; @@ -29,9 +29,10 @@ export default function EditPhoneNumber() { const phone = useGoogleMCPhoneNumber(); usePhoneNumberCheckTrackEventEffect( phone ); + useLayout( 'full-content' ); return ( - + <> - + ); } diff --git a/js/src/settings/edit-store-address.js b/js/src/settings/edit-store-address.js index 841c3e9805..1ac5833405 100644 --- a/js/src/settings/edit-store-address.js +++ b/js/src/settings/edit-store-address.js @@ -11,8 +11,8 @@ import { Flex } from '@wordpress/components'; */ import { getSettingsUrl } from '.~/utils/urls'; import { useAppDispatch } from '.~/data'; +import useLayout from '.~/hooks/useLayout'; import useStoreAddress from '.~/hooks/useStoreAddress'; -import FullContainer from '.~/components/full-container'; import TopBar from '.~/components/stepper/top-bar'; import HelpIconButton from '.~/components/help-icon-button'; import Section from '.~/wcdl/section'; @@ -31,6 +31,8 @@ const learnMoreUrl = * @see StoreAddressCard */ export default function EditStoreAddress() { + useLayout( 'full-content' ); + const { updateGoogleMCContactInformation } = useAppDispatch(); const { data: address } = useStoreAddress(); const [ isSaving, setSaving ] = useState( false ); @@ -46,7 +48,7 @@ export default function EditStoreAddress() { address.isAddressFilled && address.isMCAddressDifferent; return ( - + <> - + ); } From 952921a26c3e66b3d0e5c070fdd1bb6c31131114 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Tue, 18 Jan 2022 18:25:00 +0800 Subject: [PATCH 7/8] Replace all FullScreen usages with useLayout --- js/src/components/full-screen/README.md | 13 ------- js/src/components/full-screen/index.js | 50 ------------------------- js/src/setup-ads/index.js | 10 ++--- js/src/setup-mc/index.js | 8 ++-- 4 files changed, 9 insertions(+), 72 deletions(-) delete mode 100644 js/src/components/full-screen/README.md delete mode 100644 js/src/components/full-screen/index.js diff --git a/js/src/components/full-screen/README.md b/js/src/components/full-screen/README.md deleted file mode 100644 index d93255fd06..0000000000 --- a/js/src/components/full-screen/README.md +++ /dev/null @@ -1,13 +0,0 @@ -# FullScreen - -Make the wrapped component display in full screen. It hides the top bar and left side bar by applying `woocommerce-admin-full-screen` and `app-full-screen` CSS class to the `document.body`. - -`woocommerce-admin-full-screen` is an existing CSS class from the `woocommerce-admin` extension. ([Source](https://github.com/woocommerce/woocommerce-admin/blob/main/client/layout/style.scss#L47)) - -## Usage - -```jsx - - - -``` diff --git a/js/src/components/full-screen/index.js b/js/src/components/full-screen/index.js deleted file mode 100644 index 7b596579bd..0000000000 --- a/js/src/components/full-screen/index.js +++ /dev/null @@ -1,50 +0,0 @@ -/** - * External dependencies - */ -import { useEffect } from '@wordpress/element'; - -const cssClassNames = { - isWPToolbarDisabled: 'is-wp-toolbar-disabled', - woocommerceAdminFullScreen: 'woocommerce-admin-full-screen', - appFullScreen: 'app-full-screen', -}; - -const FullScreen = ( props ) => { - const { children } = props; - - useEffect( () => { - /** - * For WC Navigation, it already has "is-wp-toolbar-disabled" applied, - * we check this so that we do not remove it in the cleanup function. - */ - const isWPToolbarDisabled = document.body.classList.contains( - cssClassNames.isWPToolbarDisabled - ); - - if ( ! isWPToolbarDisabled ) { - document.body.classList.add( cssClassNames.isWPToolbarDisabled ); - } - - document.body.classList.add( - cssClassNames.woocommerceAdminFullScreen, - cssClassNames.appFullScreen - ); - - return () => { - if ( ! isWPToolbarDisabled ) { - document.body.classList.remove( - cssClassNames.isWPToolbarDisabled - ); - } - - document.body.classList.remove( - cssClassNames.woocommerceAdminFullScreen, - cssClassNames.appFullScreen - ); - }; - }, [] ); - - return children; -}; - -export default FullScreen; diff --git a/js/src/setup-ads/index.js b/js/src/setup-ads/index.js index 6480cddc7e..f191ada062 100644 --- a/js/src/setup-ads/index.js +++ b/js/src/setup-ads/index.js @@ -1,15 +1,13 @@ /** * Internal dependencies */ -import FullScreen from '.~/components/full-screen'; +import useLayout from '.~/hooks/useLayout'; import SetupAdsForm from './setup-ads-form'; const SetupAds = () => { - return ( - - - - ); + useLayout( 'full-page' ); + + return ; }; export default SetupAds; diff --git a/js/src/setup-mc/index.js b/js/src/setup-mc/index.js index 6df88cb5c8..dbb53febcf 100644 --- a/js/src/setup-mc/index.js +++ b/js/src/setup-mc/index.js @@ -1,16 +1,18 @@ /** * Internal dependencies */ -import FullScreen from '.~/components/full-screen'; +import useLayout from '.~/hooks/useLayout'; import SetupMCTopBar from './top-bar'; import SetupStepper from './setup-stepper'; const SetupMC = () => { + useLayout( 'full-page' ); + return ( - + <> - + ); }; From 843e4844347ab7bc4cb7a2b644027088697fadac Mon Sep 17 00:00:00 2001 From: Eason Su Date: Wed, 19 Jan 2022 11:14:35 +0800 Subject: [PATCH 8/8] Add code comments for why filter classes --- js/src/hooks/useLayout.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/js/src/hooks/useLayout.js b/js/src/hooks/useLayout.js index 9487965145..f7a70d25c5 100644 --- a/js/src/hooks/useLayout.js +++ b/js/src/hooks/useLayout.js @@ -29,6 +29,10 @@ export default function useLayout( layoutName ) { } const bodyClassList = document.body.classList; + /** + * For WC Navigation, it already has classes applied, for example, "is-wp-toolbar-disabled". + * Here filter existing classes out to avoid them being removed in the cleanup function. + */ const classNames = classNameDict[ layoutName ].filter( ( name ) => ! bodyClassList.contains( name ) );