From 0f749189ae25e681523af98d30d15ee0ccbbf12f Mon Sep 17 00:00:00 2001 From: Ben Bowler Date: Wed, 17 Jul 2024 15:04:48 +0100 Subject: [PATCH 1/4] Implement the loading state for the Audience Tile. --- .../AudienceTile/AudienceTileLoading.js | 36 ++ .../AudienceTilesWidget/AudienceTile/index.js | 18 +- .../AudienceTile/index.stories.js | 10 + .../AudienceTilesWidget/AudienceTiles.js | 4 +- .../__snapshots__/index.test.js.snap | 370 ++++++++++++++++++ .../dashboard/AudienceTilesWidget/index.js | 31 +- .../AudienceTilesWidget/index.test.js | 109 ++++-- .../analytics-4/datastore/audiences.js | 41 ++ .../analytics-4/datastore/audiences.test.js | 108 ++++- .../sass/components/analytics-4/_index.scss | 1 + ...it-audience-segmentation-tile-loading.scss | 31 ++ ...udienceTile_Loading_0_document_0_small.png | Bin 0 -> 2764 bytes ...dienceTile_Loading_0_document_1_medium.png | Bin 0 -> 6789 bytes ...udienceTile_Loading_0_document_2_large.png | Bin 0 -> 8032 bytes 14 files changed, 715 insertions(+), 44 deletions(-) create mode 100644 assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTileLoading.js create mode 100644 assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/__snapshots__/index.test.js.snap create mode 100644 assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-segmentation-tile-loading.scss create mode 100644 tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_0_small.png create mode 100644 tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_1_medium.png create mode 100644 tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_2_large.png diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTileLoading.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTileLoading.js new file mode 100644 index 00000000000..d505ade8399 --- /dev/null +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTileLoading.js @@ -0,0 +1,36 @@ +/** + * AudienceTileLoading component. + * + * Site Kit by Google, Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Internal dependencies + */ +import PreviewBlock from '../../../../../../../components/PreviewBlock'; + +export default function AudienceTileLoading() { + return ( +
+ + + + + + + +
+ ); +} diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.js index d3ca2243fe0..f25ef05f511 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.js @@ -46,7 +46,6 @@ import AudienceMetricIconTopContent from '../../../../../../../../svg/icons/audi import AudienceTileMetric from './AudienceTileMetric'; import AudienceTileCitiesMetric from './AudienceTileCitiesMetric'; import AudienceTilePagesMetric from './AudienceTilePagesMetric'; -import PreviewBlock from '../../../../../../../components/PreviewBlock'; import ChangeBadge from '../../../../../../../components/ChangeBadge'; import InfoTooltip from '../../../../../../../components/InfoTooltip'; import PartialDataBadge from './PartialDataBadge'; @@ -54,6 +53,7 @@ import PartialDataNotice from './PartialDataNotice'; import { numFmt } from '../../../../../../../util'; import AudienceTileCollectingData from './AudienceTileCollectingData'; import AudienceTileCollectingDataHideable from './AudienceTileCollectingDataHideable'; +import AudienceTileLoading from './AudienceTileLoading'; // TODO: as part of #8484 the report props should be updated to expect // the full report rows for the current tile to reduce data manipulation @@ -72,6 +72,7 @@ export default function AudienceTile( { topContentTitles, Widget, audienceResourceName, + isLoading, isZeroData, isPartialData, isTileHideable, @@ -107,9 +108,17 @@ export default function AudienceTile( { breakpoint ); - // TODO: Loading states will be implemented as part of https://github.com/google/site-kit-wp/issues/8145. - if ( ! loaded || isZeroData === undefined || isPartialData === undefined ) { - return ; + if ( + ! loaded || + isLoading || + isZeroData === undefined || + isPartialData === undefined + ) { + return ( + + + + ); } if ( isPartialData && isZeroData ) { @@ -283,6 +292,7 @@ AudienceTile.propTypes = { topContentTitles: PropTypes.object, Widget: PropTypes.elementType.isRequired, audienceResourceName: PropTypes.string.isRequired, + isLoading: PropTypes.bool, isZeroData: PropTypes.bool, isPartialData: PropTypes.bool, isTileHideable: PropTypes.bool, diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.stories.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.stories.js index e9b67fec88c..10d849797b4 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.stories.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.stories.js @@ -199,6 +199,16 @@ ReadyLongCityNames.scenario = { label: 'Modules/Analytics4/Components/AudienceSegmentation/Dashboard/AudienceTile/ReadyLongCityNames', }; +export const Loading = Template.bind( {} ); +Loading.storyName = 'Loading'; +Loading.args = { + ...readyProps, + isLoading: true, +}; +Loading.scenario = { + label: 'Modules/Analytics4/Components/AudienceSegmentation/Dashboard/AudienceTile/Loading', +}; + export const NoData = Template.bind( {} ); NoData.storyName = 'NoData'; NoData.args = { diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTiles.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTiles.js index e1340c2c523..adeb2fc9e39 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTiles.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTiles.js @@ -59,7 +59,7 @@ const hasZeroDataForAudience = ( report, audienceResourceName ) => { return totalUsers === 0; }; -export default function AudienceTiles( { Widget } ) { +export default function AudienceTiles( { Widget, widgetLoading } ) { const [ activeTile, setActiveTile ] = useState( 0 ); const breakpoint = useBreakpoint(); const isTabbedBreakpoint = @@ -300,6 +300,7 @@ export default function AudienceTiles( { Widget } ) { }, [ audiencesToClearDismissal, dismissItem, isDismissingItem ] ); const loading = + widgetLoading || ! reportLoaded || ! totalPageviewsReportLoaded || ! topCitiesReportLoaded || @@ -515,4 +516,5 @@ export default function AudienceTiles( { Widget } ) { AudienceTiles.propTypes = { Widget: PropTypes.elementType.isRequired, + widgetLoading: PropTypes.bool.isRequired, }; diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/__snapshots__/index.test.js.snap b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/__snapshots__/index.test.js.snap new file mode 100644 index 00000000000..cd7038ec757 --- /dev/null +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/__snapshots__/index.test.js.snap @@ -0,0 +1,370 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`AudienceTilesWidget should render when all configured audiences are matching available audiences 1`] = ` +
+
+
+
+
+
+
+ + +
+
+
+
+
+
+
+
+
+
+ +

+ Site Kit is collecting data for this group. +

+

+ You can hide this group until data is available. +

+ +
+
+
+
+
+
+
+
+
+`; + +exports[`AudienceTilesWidget should render when configured audience is matching available audiences 1`] = ` +
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+ +

+ Site Kit is collecting data for this group. +

+
+
+
+
+
+
+
+
+
+`; + +exports[`AudienceTilesWidget should render when some configured audiences are matching available audiences 1`] = ` +
+
+
+
+
+
+
+ + +
+
+
+
+
+
+
+
+
+
+ +

+ Site Kit is collecting data for this group. +

+

+ You can hide this group until data is available. +

+ +
+
+
+
+
+
+
+
+
+`; diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.js index 16e7db33323..0c81e558fcf 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.js @@ -20,14 +20,16 @@ * External dependencies */ import PropTypes from 'prop-types'; +import { useEffect, useState } from '@wordpress/element'; /** * Internal dependencies */ -import { useSelect } from 'googlesitekit-data'; +import { useDispatch, useSelect } from 'googlesitekit-data'; import whenActive from '../../../../../../util/when-active'; import { MODULES_ANALYTICS_4 } from '../../../../datastore/constants'; import AudienceTiles from './AudienceTiles'; +import { useInView } from '../../../../../../hooks/useInView'; function AudienceTilesWidget( { Widget, WidgetNull } ) { const availableAudiences = useSelect( ( select ) => { @@ -38,15 +40,36 @@ function AudienceTilesWidget( { Widget, WidgetNull } ) { select( MODULES_ANALYTICS_4 ).getConfiguredAudiences() ); + const [ availableAudiencesSynced, setAvailableAudiencesSynced ] = + useState( false ); + const { maybeSyncAvailableAudiences } = useDispatch( MODULES_ANALYTICS_4 ); + + const inView = useInView(); + useEffect( () => { + if ( inView && ! availableAudiencesSynced ) { + maybeSyncAvailableAudiences(); + setAvailableAudiencesSynced( true ); + } + }, [ inView, availableAudiencesSynced, maybeSyncAvailableAudiences ] ); + const hasMatchingAudience = configuredAudiences?.some( ( audience ) => availableAudiences?.includes( audience ) ); - if ( hasMatchingAudience ) { - return ; + if ( ! hasMatchingAudience ) { + return ; } - return ; + return ( + + ); } AudienceTilesWidget.propTypes = { diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.test.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.test.js index fc66fd75549..491b09c2396 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.test.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.test.js @@ -61,44 +61,59 @@ describe( 'AudienceTilesWidget', () => { jest.clearAllMocks(); } ); - it( 'should not render when availableAudiences and configuredAudiences are not loaded', () => { + it( 'should not render when availableAudiences and configuredAudiences are not loaded', async () => { muteFetch( audienceSettingsRegExp ); - const { container } = render( , { - registry, - } ); + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container ).toBeEmptyDOMElement(); } ); - it( 'should not render when availableAudiences is not loaded', () => { + it( 'should not render when availableAudiences is not loaded', async () => { registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetAudienceSettings( { configuredAudiences: [ 'properties/12345/audiences/1' ], isAudienceSegmentationWidgetHidden: false, } ); - const { container } = render( , { - registry, - } ); + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container ).toBeEmptyDOMElement(); } ); - it( 'should not render when configuredAudiences is not loaded', () => { + it( 'should not render when configuredAudiences is not loaded', async () => { muteFetch( audienceSettingsRegExp ); registry .dispatch( MODULES_ANALYTICS_4 ) .setAvailableAudiences( availableAudiences ); - const { container } = render( , { - registry, - } ); + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container ).toBeEmptyDOMElement(); } ); - it( 'should not render when there is no available audience', () => { + it( 'should not render when there is no available audience', async () => { registry.dispatch( MODULES_ANALYTICS_4 ).setAvailableAudiences( [] ); registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetAudienceSettings( { @@ -106,14 +121,19 @@ describe( 'AudienceTilesWidget', () => { isAudienceSegmentationWidgetHidden: false, } ); - const { container } = render( , { - registry, - } ); + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container ).toBeEmptyDOMElement(); } ); - it( 'should not render when there is no configured audience', () => { + it( 'should not render when there is no configured audience', async () => { registry .dispatch( MODULES_ANALYTICS_4 ) .setAvailableAudiences( availableAudiences ); @@ -123,14 +143,19 @@ describe( 'AudienceTilesWidget', () => { isAudienceSegmentationWidgetHidden: false, } ); - const { container } = render( , { - registry, - } ); + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container ).toBeEmptyDOMElement(); } ); - it( 'should not render when configuredAudiences is null (not set)', () => { + it( 'should not render when configuredAudiences is null (not set)', async () => { registry .dispatch( MODULES_ANALYTICS_4 ) .setAvailableAudiences( availableAudiences ); @@ -140,14 +165,19 @@ describe( 'AudienceTilesWidget', () => { isAudienceSegmentationWidgetHidden: false, } ); - const { container } = render( , { - registry, - } ); + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container ).toBeEmptyDOMElement(); } ); - it( 'should not render when there is no matching audience', () => { + it( 'should not render when there is no matching audience', async () => { registry .dispatch( MODULES_ANALYTICS_4 ) .setAvailableAudiences( availableAudiences ); @@ -157,14 +187,23 @@ describe( 'AudienceTilesWidget', () => { isAudienceSegmentationWidgetHidden: false, } ); - const { container } = render( , { - registry, - } ); + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container ).toBeEmptyDOMElement(); } ); it( 'should render when configured audience is matching available audiences', async () => { + registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( { + availableAudiencesLastSyncedAt: ( Date.now() - 1000 ) / 1000, + } ); + registry .dispatch( MODULES_ANALYTICS_4 ) .setAvailableAudiences( availableAudiences ); @@ -183,10 +222,14 @@ describe( 'AudienceTilesWidget', () => { await waitForRegistry(); - expect( container ).not.toBeEmptyDOMElement(); + expect( container ).toMatchSnapshot(); } ); it( 'should render when all configured audiences are matching available audiences', async () => { + registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( { + availableAudiencesLastSyncedAt: ( Date.now() - 1000 ) / 1000, + } ); + registry .dispatch( MODULES_ANALYTICS_4 ) .setAvailableAudiences( availableAudiences ); @@ -208,10 +251,14 @@ describe( 'AudienceTilesWidget', () => { await waitForRegistry(); - expect( container ).not.toBeEmptyDOMElement(); + expect( container ).toMatchSnapshot(); } ); it( 'should render when some configured audiences are matching available audiences', async () => { + registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( { + availableAudiencesLastSyncedAt: ( Date.now() - 1000 ) / 1000, + } ); + registry .dispatch( MODULES_ANALYTICS_4 ) .setAvailableAudiences( availableAudiences ); @@ -233,6 +280,6 @@ describe( 'AudienceTilesWidget', () => { await waitForRegistry(); - expect( container ).not.toBeEmptyDOMElement(); + expect( container ).toMatchSnapshot(); } ); } ); diff --git a/assets/js/modules/analytics-4/datastore/audiences.js b/assets/js/modules/analytics-4/datastore/audiences.js index e15a4a79e43..eec825dbcf2 100644 --- a/assets/js/modules/analytics-4/datastore/audiences.js +++ b/assets/js/modules/analytics-4/datastore/audiences.js @@ -172,6 +172,47 @@ const baseActions = { return { response, error }; }, + /** + * Syncs available audiences older than 1 hour. + * + * @since n.e.x.t + * + * @return {void} + */ + *maybeSyncAvailableAudiences() { + const registry = yield commonActions.getRegistry(); + const { select, dispatch } = registry; + + const availableAudiencesLastSyncedAt = + select( MODULES_ANALYTICS_4 ).getAvailableAudiencesLastSyncedAt(); + + // Update the audience cache if the availableAudiencesLastSyncedAt is older than 1 hour. + if ( + ! availableAudiencesLastSyncedAt || + availableAudiencesLastSyncedAt * 1000 < + // eslint-disable-next-line sitekit/no-direct-date + Date.now() - 1 * 60 * 60 * 1000 + ) { + const { response: availableAudiences } = yield commonActions.await( + dispatch( MODULES_ANALYTICS_4 ).syncAvailableAudiences() + ); + + // Remove any configuredAudiences that are no longer available in availableAudiences. + const configuredAudiences = + select( MODULES_ANALYTICS_4 ).getConfiguredAudiences(); + + const newConfiguredAudiences = configuredAudiences?.filter( + ( configuredAudience ) => + availableAudiences?.some( + ( { name } ) => name === configuredAudience + ) + ); + dispatch( MODULES_ANALYTICS_4 ).setConfiguredAudiences( + newConfiguredAudiences || [] + ); + } + }, + /** * Populates the configured audiences with the top two user audiences and/or the Site Kit-created audiences, * depending on their availability and suitability (data over the last 90 days is required for user audiences). diff --git a/assets/js/modules/analytics-4/datastore/audiences.test.js b/assets/js/modules/analytics-4/datastore/audiences.test.js index d268284060a..fa94e755957 100644 --- a/assets/js/modules/analytics-4/datastore/audiences.test.js +++ b/assets/js/modules/analytics-4/datastore/audiences.test.js @@ -53,6 +53,9 @@ describe( 'modules/analytics-4 audiences', () => { const syncAvailableAudiencesEndpoint = new RegExp( '^/google-site-kit/v1/modules/analytics-4/data/sync-audiences' ); + const audienceSettingsEndpoint = new RegExp( + '^/google-site-kit/v1/modules/analytics-4/data/audience-settings' + ); const audience = { displayName: 'Recently active users', @@ -304,6 +307,107 @@ describe( 'modules/analytics-4 audiences', () => { } ); } ); + describe( 'maybeSyncAvailableAudiences', () => { + it( 'should call syncAvailableAudiences if the availableAudiencesLastSyncedAt setting is undefined', async () => { + fetchMock.post( syncAvailableAudiencesEndpoint, { + body: availableAudiencesFixture, + status: 200, + } ); + + await registry + .dispatch( MODULES_ANALYTICS_4 ) + .maybeSyncAvailableAudiences(); + + expect( fetchMock ).toHaveFetchedTimes( 1 ); + expect( fetchMock ).toHaveFetched( + syncAvailableAudiencesEndpoint + ); + } ); + + it( 'should not call syncAvailableAudiences if the availableAudiencesLastSyncedAt is within the last hour', async () => { + fetchMock.post( syncAvailableAudiencesEndpoint, { + body: availableAudiencesFixture, + status: 200, + } ); + + registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( { + availableAudiencesLastSyncedAt: + ( Date.now() - 1000 ) / 1000, // Value expected to be a PHP date so divide by 1000. + } ); + + await registry + .dispatch( MODULES_ANALYTICS_4 ) + .maybeSyncAvailableAudiences(); + + expect( fetchMock ).toHaveFetchedTimes( 0 ); + } ); + + it( 'should call syncAvailableAudiences if the availableAudiencesLastSyncedAt is not within the last hour', async () => { + fetchMock.post( syncAvailableAudiencesEndpoint, { + body: availableAudiencesFixture, + status: 200, + } ); + + registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( { + availableAudiencesLastSyncedAt: + ( Date.now() - 2 * 60 * 60 * 1000 ) / 1000, // Value expected to be a PHP date so divide by 1000. + } ); + + await registry + .dispatch( MODULES_ANALYTICS_4 ) + .maybeSyncAvailableAudiences(); + + expect( fetchMock ).toHaveFetchedTimes( 1 ); + expect( fetchMock ).toHaveFetched( + syncAvailableAudiencesEndpoint + ); + } ); + + it( 'should remove configured audiences which are no longer available', async () => { + const availableAudiencesSubset = [ + availableAudiencesFixture[ 0 ], + availableAudiencesFixture[ 2 ], + ]; + + fetchMock.post( syncAvailableAudiencesEndpoint, { + body: availableAudiencesSubset, + status: 200, + } ); + + const settings = { + configuredAudiences: availableAudiencesFixture.reduce( + ( acc, a ) => [ ...acc, a.name ], + [] + ), + isAudienceSegmentationWidgetHidden: false, + }; + + registry + .dispatch( MODULES_ANALYTICS_4 ) + .receiveGetAudienceSettings( settings ); + + await registry + .dispatch( MODULES_ANALYTICS_4 ) + .maybeSyncAvailableAudiences(); + + expect( fetchMock ).toHaveFetchedTimes( 1 ); + expect( fetchMock ).toHaveFetched( + syncAvailableAudiencesEndpoint + ); + + expect( + registry + .select( MODULES_ANALYTICS_4 ) + .getConfiguredAudiences() + ).toEqual( + availableAudiencesSubset.reduce( + ( acc, a ) => [ ...acc, a.name ], + [] + ) + ); + } ); + } ); + describe( 'enableAudienceGroup', () => { function createAvailableUserAudience( audienceID ) { return { @@ -408,10 +512,6 @@ describe( 'modules/analytics-4 audiences', () => { }; } - const audienceSettingsEndpoint = new RegExp( - '^/google-site-kit/v1/modules/analytics-4/data/audience-settings' - ); - const testPropertyID = propertiesFixture[ 0 ]._id; const referenceDate = '2024-05-10'; diff --git a/assets/sass/components/analytics-4/_index.scss b/assets/sass/components/analytics-4/_index.scss index e139b957ad3..57e85b51094 100644 --- a/assets/sass/components/analytics-4/_index.scss +++ b/assets/sass/components/analytics-4/_index.scss @@ -23,6 +23,7 @@ @import "googlesitekit-analytics-enhanced-measurement"; @import "audience-segmentation/googlesitekit-audience-segmentation-setup-cta"; @import "audience-segmentation/googlesitekit-audience-segmentation-tile"; +@import "audience-segmentation/googlesitekit-audience-segmentation-tile-loading"; @import "audience-segmentation/googlesitekit-audience-segmentation-tile-error"; @import "audience-segmentation/googlesitekit-audience-segmentation-info-notice"; @import "audience-segmentation/googlesitekit-audience-segmentation-error"; diff --git a/assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-segmentation-tile-loading.scss b/assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-segmentation-tile-loading.scss new file mode 100644 index 00000000000..bf69057b2f9 --- /dev/null +++ b/assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-segmentation-tile-loading.scss @@ -0,0 +1,31 @@ +/** + * Audience Segmentation Loading styles. + * + * Site Kit by Google, Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +.googlesitekit-plugin { + .googlesitekit-audience-segmentation-tile-loading { + margin: 24px 24px 28px; + + .googlesitekit-preview-block { + margin: 12px 0; + + &:first-of-type { + margin-bottom: 24px; + } + } + } +} diff --git a/tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_0_small.png b/tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_0_small.png new file mode 100644 index 0000000000000000000000000000000000000000..6727892b0994f9f85b696a41f052ef130102aa1b GIT binary patch literal 2764 zcmeAS@N?(olHy`uVBq!ia0y~yU|hn$z~sWg1{8^W5TFI57>k44ofy`glX=O&z}4pI z;uumf=k49odAALi90E_3YD?7#iq`-8z|W@+s4mnBlmFK4cKoxK0~ zuM47=9u^2FHK=a`8c_ceD0Tet$HeDV9uuaeO4`@`k=T8AdYmB314p|pv>@UG*h584dIQ3>Hgdf-vm~;O5#{2Jqip13#)IV?R{#9aS`={)TbF;~@|EsEOkh1r5 zaSW-L^Y-rPJZVc22ghwX-X8z{yYpI5(Soo2@Y!#>JSXU$Hmdvc!*JW}^1tpX3=9WM)+cVenRD#@ z{r&kyy0hbaW}i*_^XF%Aj@dhIpzU`1fB*hnSYB>EZ{ECPPm5;m`gSENYMN^gP%8f1 z>j#4{rExzXT`)}QyyLZn<@qyg6bN6m(punxF;HEcUv#NnXwm$Ef z@2B9Voc;HA-;x75P%JJ#e$K;J3dXX`3=I2bM+R?N-F&7x8R)Wdlf|v;_>AQ1fswaw zv7~N_6Oi#i-0n!rneyYHz|U~w_T6~}sO`b-p90gXfD!VcoJvtad1Q{7K&@;s8j_^Gkl?ffMGY*@`d+atEi z>O@>z+&ViJVB0L+7}Pe~^!aDaS4E(--E2hj%w@vb*H(SUA75n$HWGj6q?ulrlKq~) z{cukzNMUOvuZijjpOc{W+oJ*tpWlD3Z{c^A=VO0A8Puk{b^7$_D~VUOC;%I@Humycj~_4I zcLQBFy#Ux)e*E~c??2uhbN4;^5!}DO?yuDDyLoTE|K6x`T6Y`JsvR>+V~;;9*zuCJ z&75;n`0>YubNrTXy#4lA-ge#=O33??Y;faoPX1QtGRyfe?12IcGF8pv-CfQ5zxaO z3c!%<7-f!z!)PKH%?zU@!Dxv%S{;m5i=%CV(YDcO!*H}!JlZNAa;@S&%*Xrw{Ou{} S1Ws%)FnGH9xvXpdTSe*V|X z^bLF;SJ?DD{;07Jt07pOVv4TbglHV%%-<(XI_>FB>^QGj+pr7>n24`E3}($ zUOnBn>5Pf*R)9ts%E?>U9<8I!|H(R=VE~^{^*(rbQ^&^kkIbKn2MkX zK(471zdG&A-R7R+WFV*EzUM`wcXBD;e*fL;4&*cJnY}3W1~_JZoRiauH=6cF)7}8L&_*-i zXeJz9nQ(XYgvje(?j-G{!itmGrQI$M^3Bm2esjUq=p{$Shv@1{`sSzjzL3wkZF&~i5rtZ za(25u|C|$3Y3nAEa_q;q#dFj_U9TD!eSQ6xMxJ6R*Ggs2pFe*x_8w4_!Twd*?%w6g zm*-fg^%$J!`S_OI&fb2xy#P?^!T&X{XC0qm_(;L{*mltc$B(qNb(B=uI!Q zkvsmeLL?0+->|!KFKbogw6p!eH%)-n=1;YnfBtcSMbFJ$85S%Hinw?|8k(UN0$l^g>*Rx+BO zM)T8Xej05K4~~)B(b8zNG#V_W(VxqGvfQB7DbJ_v^Ny}N1Z=4%sDbA-(wsnT-%-wJ n7>uTa(Y(OGFd7EKFbo(^ul(~jT%cSPR0Vjt`njxgN@xNAp79?w literal 0 HcmV?d00001 From 9de9eefbd556bc604e0d2b15cb3b5db2d3d8e916 Mon Sep 17 00:00:00 2001 From: Ben Bowler Date: Mon, 22 Jul 2024 13:08:34 +0100 Subject: [PATCH 2/4] Update PreviewBlock styles to better match height when other components are loaded. --- .../AudienceTile/AudienceTileLoading.js | 3 ++- ...it-audience-segmentation-tile-loading.scss | 25 ++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTileLoading.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTileLoading.js index d505ade8399..298d51957f5 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTileLoading.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTileLoading.js @@ -24,7 +24,8 @@ import PreviewBlock from '../../../../../../../components/PreviewBlock'; export default function AudienceTileLoading() { return (
- + { /* The first preview block is only visible on desktop to preview the header which is hidden on other screens. */ } + diff --git a/assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-segmentation-tile-loading.scss b/assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-segmentation-tile-loading.scss index bf69057b2f9..557357ad96e 100644 --- a/assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-segmentation-tile-loading.scss +++ b/assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-segmentation-tile-loading.scss @@ -18,13 +18,32 @@ .googlesitekit-plugin { .googlesitekit-audience-segmentation-tile-loading { - margin: 24px 24px 28px; + margin: $grid-gap-phone $grid-gap-phone $grid-gap-phone + 4px; + + @media (min-width: $bp-desktop) { + margin: $grid-gap-desktop $grid-gap-desktop $grid-gap-desktop + 4px; + } .googlesitekit-preview-block { - margin: 12px 0; + margin: 22px 0; &:first-of-type { - margin-bottom: 24px; + display: none; + } + + &:last-of-type { + // Match the no data height of the final metric block + // on tablet and desktop to minimize layout shift. + margin-bottom: 31px; + } + + @media (min-width: $bp-desktop) { + margin: 24px 0; + + &:first-of-type { + display: flex; + margin-bottom: 35px; + } } } } From 73a94f7982ecfe4958c9604ffc3e8547227ad1e4 Mon Sep 17 00:00:00 2001 From: Ben Bowler Date: Mon, 22 Jul 2024 15:05:42 +0100 Subject: [PATCH 3/4] Address code review comments improving loading and sync behaviour. --- .../ErrorNotice.test.js | 11 ++ .../AudienceTile/index.stories.js | 10 ++ .../AudienceTilesWidget/AudienceTiles.js | 4 +- .../analytics-4/datastore/audiences.js | 49 +++--- .../analytics-4/datastore/audiences.test.js | 144 ++++++++++++------ ...udienceTile_Loading_0_document_0_small.png | Bin 2764 -> 2912 bytes ...dienceTile_Loading_0_document_1_medium.png | Bin 6789 -> 6915 bytes ...udienceTile_Loading_0_document_2_large.png | Bin 8032 -> 8217 bytes 8 files changed, 151 insertions(+), 67 deletions(-) diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/ErrorNotice.test.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/ErrorNotice.test.js index c8625c96bcc..33bc572aa5f 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/ErrorNotice.test.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/ErrorNotice.test.js @@ -42,6 +42,9 @@ describe( 'ErrorNotice', () => { const syncAvailableAudiencesEndpoint = new RegExp( '^/google-site-kit/v1/modules/analytics-4/data/sync-audiences' ); + const audienceSettingsEndpoint = new RegExp( + '^/google-site-kit/v1/modules/analytics-4/data/audience-settings' + ); const reportOptions = { endDate: '2024-03-27', @@ -265,6 +268,14 @@ describe( 'ErrorNotice', () => { status: 200, } ); + fetchMock.getOnce( audienceSettingsEndpoint, { + body: { + data: { + configuredAudiences: [], + }, + }, + } ); + expect( registry .select( MODULES_ANALYTICS_4 ) diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.stories.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.stories.js index 10d849797b4..95f18b9d9f1 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.stories.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.stories.js @@ -205,6 +205,16 @@ Loading.args = { ...readyProps, isLoading: true, }; +Loading.decorators = [ + ( Story ) => { + // Ensure the animation is paused for VRT tests to correctly capture the loading state. + return ( +
+ +
+ ); + }, +]; Loading.scenario = { label: 'Modules/Analytics4/Components/AudienceSegmentation/Dashboard/AudienceTile/Loading', }; diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTiles.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTiles.js index adeb2fc9e39..f372b60b4ee 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTiles.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTiles.js @@ -248,7 +248,7 @@ export default function AudienceTiles( { Widget, widgetLoading } ) { const visible = []; const tempAudiences = configuredAudiences.slice(); - while ( reportLoaded && tempAudiences.length > 0 ) { + while ( tempAudiences.length > 0 ) { const audienceResourceName = tempAudiences.shift(); const isDismissed = dismissedItems?.includes( @@ -278,7 +278,7 @@ export default function AudienceTiles( { Widget, widgetLoading } ) { } return [ toClear, visible ]; - }, [ configuredAudiences, dismissedItems, reportLoaded, report ] ); + }, [ configuredAudiences, dismissedItems, report ] ); // Re-dismiss with a short expiry time to clear any previously dismissed tiles. // This ensures that the tile will reappear when it is populated with data again. diff --git a/assets/js/modules/analytics-4/datastore/audiences.js b/assets/js/modules/analytics-4/datastore/audiences.js index eec825dbcf2..16c59331848 100644 --- a/assets/js/modules/analytics-4/datastore/audiences.js +++ b/assets/js/modules/analytics-4/datastore/audiences.js @@ -166,10 +166,37 @@ const baseActions = { * @return {Object} Object with `response` and `error`. */ *syncAvailableAudiences() { - const { response, error } = + const { response: availableAudiences, error } = yield fetchSyncAvailableAudiencesStore.actions.fetchSyncAvailableAudiences(); - return { response, error }; + if ( error ) { + return { response: availableAudiences, error }; + } + + const registry = yield commonActions.getRegistry(); + const { select, dispatch } = registry; + + // Remove any configuredAudiences that are no longer available in availableAudiences. + const configuredAudiences = + select( MODULES_ANALYTICS_4 ).getConfiguredAudiences(); + const newConfiguredAudiences = configuredAudiences?.filter( + ( configuredAudience ) => + availableAudiences?.some( + ( { name } ) => name === configuredAudience + ) + ); + + if ( + configuredAudiences && + newConfiguredAudiences && + newConfiguredAudiences !== configuredAudiences + ) { + dispatch( MODULES_ANALYTICS_4 ).setConfiguredAudiences( + newConfiguredAudiences || [] + ); + } + + return { response: availableAudiences, error }; }, /** @@ -186,30 +213,16 @@ const baseActions = { const availableAudiencesLastSyncedAt = select( MODULES_ANALYTICS_4 ).getAvailableAudiencesLastSyncedAt(); - // Update the audience cache if the availableAudiencesLastSyncedAt is older than 1 hour. + // Update the audience cache if the availableAudiencesLastSyncedAt setting is older than 1 hour. if ( ! availableAudiencesLastSyncedAt || availableAudiencesLastSyncedAt * 1000 < // eslint-disable-next-line sitekit/no-direct-date Date.now() - 1 * 60 * 60 * 1000 ) { - const { response: availableAudiences } = yield commonActions.await( + yield commonActions.await( dispatch( MODULES_ANALYTICS_4 ).syncAvailableAudiences() ); - - // Remove any configuredAudiences that are no longer available in availableAudiences. - const configuredAudiences = - select( MODULES_ANALYTICS_4 ).getConfiguredAudiences(); - - const newConfiguredAudiences = configuredAudiences?.filter( - ( configuredAudience ) => - availableAudiences?.some( - ( { name } ) => name === configuredAudience - ) - ); - dispatch( MODULES_ANALYTICS_4 ).setConfiguredAudiences( - newConfiguredAudiences || [] - ); } }, diff --git a/assets/js/modules/analytics-4/datastore/audiences.test.js b/assets/js/modules/analytics-4/datastore/audiences.test.js index fa94e755957..ed08c7f4cff 100644 --- a/assets/js/modules/analytics-4/datastore/audiences.test.js +++ b/assets/js/modules/analytics-4/datastore/audiences.test.js @@ -270,10 +270,20 @@ describe( 'modules/analytics-4 audiences', () => { status: 500, } ); + fetchMock.getOnce( audienceSettingsEndpoint, { + body: { + data: { + configuredAudiences: [], + }, + }, + } ); + const { response, error } = await registry .dispatch( MODULES_ANALYTICS_4 ) .syncAvailableAudiences(); + await waitForDefaultTimeouts(); + expect( response ).toBeUndefined(); expect( error ).toEqual( errorResponse ); @@ -292,10 +302,20 @@ describe( 'modules/analytics-4 audiences', () => { status: 200, } ); + fetchMock.get( audienceSettingsEndpoint, { + body: { + data: { + configuredAudiences: [], + }, + }, + } ); + const { response, error } = await registry .dispatch( MODULES_ANALYTICS_4 ) .syncAvailableAudiences(); + await waitForDefaultTimeouts(); + expect( response ).toEqual( availableAudiences ); expect( error ).toBeUndefined(); @@ -305,44 +325,80 @@ describe( 'modules/analytics-4 audiences', () => { .getAvailableAudiences() ).toEqual( availableAudiences ); } ); - } ); - describe( 'maybeSyncAvailableAudiences', () => { - it( 'should call syncAvailableAudiences if the availableAudiencesLastSyncedAt setting is undefined', async () => { + it( 'should remove configured audiences which are no longer available', async () => { + const availableAudiencesSubset = [ + availableAudiencesFixture[ 0 ], + availableAudiencesFixture[ 2 ], + ]; + fetchMock.post( syncAvailableAudiencesEndpoint, { - body: availableAudiencesFixture, + body: availableAudiencesSubset, status: 200, } ); + const settings = { + configuredAudiences: availableAudiencesFixture.reduce( + ( acc, { name } ) => [ ...acc, name ], + [] + ), + isAudienceSegmentationWidgetHidden: false, + }; + + registry + .dispatch( MODULES_ANALYTICS_4 ) + .receiveGetAudienceSettings( settings ); + await registry .dispatch( MODULES_ANALYTICS_4 ) - .maybeSyncAvailableAudiences(); + .syncAvailableAudiences(); expect( fetchMock ).toHaveFetchedTimes( 1 ); expect( fetchMock ).toHaveFetched( syncAvailableAudiencesEndpoint ); + + expect( + registry + .select( MODULES_ANALYTICS_4 ) + .getConfiguredAudiences() + ).toEqual( + availableAudiencesSubset.reduce( + ( acc, { name } ) => [ ...acc, name ], + [] + ) + ); } ); + } ); - it( 'should not call syncAvailableAudiences if the availableAudiencesLastSyncedAt is within the last hour', async () => { - fetchMock.post( syncAvailableAudiencesEndpoint, { + describe( 'maybeSyncAvailableAudiences', () => { + it( 'should call syncAvailableAudiences if the availableAudiencesLastSyncedAt setting is undefined', async () => { + fetchMock.postOnce( syncAvailableAudiencesEndpoint, { body: availableAudiencesFixture, status: 200, } ); - registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( { - availableAudiencesLastSyncedAt: - ( Date.now() - 1000 ) / 1000, // Value expected to be a PHP date so divide by 1000. + fetchMock.getOnce( audienceSettingsEndpoint, { + body: { + data: { + configuredAudiences: [], + }, + }, } ); await registry .dispatch( MODULES_ANALYTICS_4 ) .maybeSyncAvailableAudiences(); - expect( fetchMock ).toHaveFetchedTimes( 0 ); + await waitForDefaultTimeouts(); + + expect( fetchMock ).toHaveFetchedTimes( 2 ); + expect( fetchMock ).toHaveFetched( + syncAvailableAudiencesEndpoint + ); } ); - it( 'should call syncAvailableAudiences if the availableAudiencesLastSyncedAt is not within the last hour', async () => { + it( 'should not call syncAvailableAudiences if the availableAudiencesLastSyncedAt setting is within the last hour', async () => { fetchMock.post( syncAvailableAudiencesEndpoint, { body: availableAudiencesFixture, status: 200, @@ -350,61 +406,45 @@ describe( 'modules/analytics-4 audiences', () => { registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( { availableAudiencesLastSyncedAt: - ( Date.now() - 2 * 60 * 60 * 1000 ) / 1000, // Value expected to be a PHP date so divide by 1000. + ( Date.now() - 1000 ) / 1000, // Value expected to be a PHP date so divide by 1000. } ); await registry .dispatch( MODULES_ANALYTICS_4 ) .maybeSyncAvailableAudiences(); - expect( fetchMock ).toHaveFetchedTimes( 1 ); - expect( fetchMock ).toHaveFetched( - syncAvailableAudiencesEndpoint - ); + expect( fetchMock ).toHaveFetchedTimes( 0 ); } ); - it( 'should remove configured audiences which are no longer available', async () => { - const availableAudiencesSubset = [ - availableAudiencesFixture[ 0 ], - availableAudiencesFixture[ 2 ], - ]; - - fetchMock.post( syncAvailableAudiencesEndpoint, { - body: availableAudiencesSubset, + it( 'should call syncAvailableAudiences if the availableAudiencesLastSyncedAt setting is not within the last hour', async () => { + fetchMock.postOnce( syncAvailableAudiencesEndpoint, { + body: availableAudiencesFixture, status: 200, } ); - const settings = { - configuredAudiences: availableAudiencesFixture.reduce( - ( acc, a ) => [ ...acc, a.name ], - [] - ), - isAudienceSegmentationWidgetHidden: false, - }; + fetchMock.getOnce( audienceSettingsEndpoint, { + body: { + data: { + configuredAudiences: [], + }, + }, + } ); - registry - .dispatch( MODULES_ANALYTICS_4 ) - .receiveGetAudienceSettings( settings ); + registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( { + availableAudiencesLastSyncedAt: + ( Date.now() - 2 * 60 * 60 * 1000 ) / 1000, // Value expected to be a PHP date so divide by 1000. + } ); await registry .dispatch( MODULES_ANALYTICS_4 ) .maybeSyncAvailableAudiences(); - expect( fetchMock ).toHaveFetchedTimes( 1 ); + await waitForDefaultTimeouts(); + + expect( fetchMock ).toHaveFetchedTimes( 2 ); expect( fetchMock ).toHaveFetched( syncAvailableAudiencesEndpoint ); - - expect( - registry - .select( MODULES_ANALYTICS_4 ) - .getConfiguredAudiences() - ).toEqual( - availableAudiencesSubset.reduce( - ( acc, a ) => [ ...acc, a.name ], - [] - ) - ); } ); } ); @@ -1105,10 +1145,20 @@ describe( 'modules/analytics-4 audiences', () => { status: 500, } ); + fetchMock.get( audienceSettingsEndpoint, { + body: { + data: { + configuredAudiences: [], + }, + }, + } ); + const { response, error } = await registry .dispatch( MODULES_ANALYTICS_4 ) .enableAudienceGroup(); + await waitForDefaultTimeouts(); + expect( response ).toBeUndefined(); expect( error ).toEqual( errorResponse ); diff --git a/tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_0_small.png b/tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_0_small.png index 6727892b0994f9f85b696a41f052ef130102aa1b..38a6c1013f7ba7fc54e46519b726001ffde34803 100644 GIT binary patch delta 976 zcmb`F|5MU;9LL{)g(#j4F8iV*$Zo%^3%w(0nU-2>t%KdwPKS}g+_`EEhBVNROqlL! zU7prHcj~52@BDTfrgC(Uu>;Iawj0D5V?gAAW5NM3Kp>b2?&JTk*U!)A^Pcy7J&e2E z1$Rr{&fS0Rbk+N@hdTPyoGMh0vph1~ zF30}JNu*N8^1ugn$d3XXd=;8p4O4PTq;`pyn@lE~l#cEMR8cFpy7qwI2PggEV9R{B zTCI+5Y;0Wh&GOQiFXYy1*VRtN3hx)J`_8wMSBK0#LFI^nw(>H`=0Bo@MCBB8>hX5O z)|xNYHpHA79R3_8->;QNJ%(O|LlP+O1kZmxJMd>Pk&5}u4Aq1~dPmVPYcP+TG(e#a zoJ?v;r_&ELnnJVL>}i=dLLru1QGfsEZ1;hrqScrd7^l~E70be_#qTN`gfqDzkwzdE z(`{?uYYpPktngGp6Jfmfk_MG(@csltePpDTT7;3*91y!qs{yj32WZvvs&RSO-@;2Qdz*^#U!5_lDey8%iyxf89wWUx-wRI3@?ZsyLHx?} z6mwz|VnbPAX|G|iV>$LIj6!Sk9yj|@J-5X~aBWC4cdL}_O|nhFJ|om zk)tN7jKzCZIm{HYcgm<&W_B70hb|HH$s5Oqo7*=(R$-gon}W?S(B4C6WcA;3bkZ1> z);4KdlugjnKbvPS#bUAP#-5U39Tw>jU0?spSqL&*3(Z6tE9FHli^Acs$M65mDF$1k z+u+9IH(>E%nX*x^ss3Ik-6A*$}o3 zBF*kMWK^%`NFesFBzCg5iMM~eG*f4__-X%y!-i(%zF-2Hu9fSkppVY5xNK`J`}lmm zCy_{4p6J@{W?aEwa4I7JlGi&%^WgU~&jNRhF~m|Nq6TmzLCJi8ZIB5E={jlfHvUSD5d%5Wfl+t1s=3=M`XoDCe41z8R16TeSYudb|=+wJv^%urX3nws z_Vsd`qGrYQoUKnYuKV}L(oFidD#JfheSQ7p-FN%8Z{H5IeOaIFR;kmQIs^~cze|2- z_`B@Pl_DVL`v<|LE0fPjy(qBgd0S?kZNczp+wHd&GJKbh%@$hHzxFOejlOYtkUuBT zMH{wCKal=)qt^WJUB^vUOh4|Wt)29aZ_;l!hBFxl?>c_^pe@T_9F@?@a^S9zw#s=~ zMuWwy!DglmANHyO$pbu-KQiu~e1WZ#y%lW_YmkEXeL%F?!|xObyS?7PE$VFx=4w?= z5Bxx9JpTIit77=!vdiu=|JOc$e}Dh+y8ZI)hZA*9f9kl*@WJ(3sVp!u7fRod6I#L# zjPW_=pKrYX9;irMtwH_s#_nGwR(+0`03N9Z@eu_-h8tsHPVis z;orW}So{9lZ_V_kpSGBPKECWbP0l+XkK9`}QH diff --git a/tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_1_medium.png b/tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_1_medium.png index 4d686a1b79e5d9f1c73c031cd11bebc99f7e9af0..e6502866221d3d72ecedaf7446bd18f5e0919b69 100644 GIT binary patch literal 6915 zcmeAS@N?(olHy`uVBq!ia0y~yU`}CRU`gR%1B%G{Uv~pijKx9jP7LeL$-HD>klO3% z;uumf=k48teo2914i~wVmSiP){Q0kMVyH4rtYeQ{$eMSF%PV$-Uh|rVTYWV`Z+h~^h(CXU=87!K z{Po*%uHWMdo4)JUuk%~igB;N*YWv<>loa`~3N{#azG5cGcIiqIATPco-Od zl-(2eFa9r^eXU3+8K|`Ut6b`4yW(~1xn{Eqi;In~KL)zrZtm>avp3$%Nvpo6O`E%b-n^|tAZ7u}y3qdREF5r|#2-7R1POOr9@USqvA838d z%HMBetM-9%se{V|V9Ie}8D%2EAtjW*VE^9L?!b`pxPcI+TS$d{W#I4MUB$w{@WAVC z{`*Uxb58)BqSOQ|%oUVInMmQVx~c(~l0mhE!=X0%2hXNe>=hMvt||j#C;U}i_4Il9 z15g0tsYi({!s%cFu$H)!{~lCJFbEAmO%AFg4?hN1k~@OF-`@SDW&jG{AyFHGYKgjP za4j)m1NCe2IkG92P0kv!0~>(Tjoi8C%BC#8{PK+=Fj?%IWf}G2jY2iB9a{~oA{*AH zf!maQ#~Fx?=W`K(aN zHeh@4z}6e@zi;0AFMFG~^ZW1B|J8xbk$qdYZ{Hp{J#aJYO%vb8PoEyGJ_)of|7D3) z@AK!+Z*EIYJ|mHm{73S`)2FG|7=d+T7SO?RKv5Z>Xyij%Tic&Mm4Mdj%Jd)q_~lE< z#CYa)Y4wkO1o!W+`z!VOYgLZf?2QpRdW9gj9QCKp9wn6TGchVSv$ucd3Q zzup+Pz7<&Get5a}zEZt~jNZTUHQj7CCH}8H^|#JGFF*fy)>bKCYhOpKd%rx;b%#uR z>+P3c&IDQc>({Tu=kYct{#}3l`gMDqB~bc&&Npvh1D)Z=e|ohghD1^xb>wI(e6$NP z+7%n^hmQ8gfwkmlbud~Tj3$E7>R_}a7_AQIQyrYBXLz$uAcn;)(G=J_WAJqKb6Mw< G&;$U_>tTuj delta 860 zcmZoRYc-u<=xFch;uumf=k49odD5054vyP&ygmN?cjvVz`?8@*tkHI}jLVdM`A~;^ z&#cOVt8bn@e3&T9z`(F)w{I~^X?|)lnZ8rOC#I&bx85tNf z*1Z1pEkC?AAhvqYd?JU+b+)ux~Glm{`@f9cDwwqy9xut0h9HK z+ivC@dw+j_eZG^PsCM-y zYM^J==RNcN6x@`v|Nib?nuj-^5dXz%W&iN-FXG5?ZNJ!0@JI2VfmqavLRdXWCgYs zDqyzyb$pvjW7Fq=YzUrt(J7q$=F_5`zXX65%|F{Cw#({7TwL5bI~Jf@(~UuH-Sqiq z%~wUBwB2lkb6qB^eQniu{P9(GV5t4jNi)4LCHp;p`{ABckiynTUK7<5J|{u`c~oHG z^ZU>BE&R^%eC+QhGu8t``PS*vr>`Vl*`ff9V;g(>-+GcjU#iCHP5=1ii%G7P*|8%E z(vQzG&YwSjxg5}83%0F${Y64LxoAS~$Mlx(KYqM;-wkx$^a5b|dHndX??2uhbN4;^ z5!}DO?yuDDyLoTE|K6x`T6Y`JX**_?#vXrIu;V3bn>pvE@Z*mQW9IlR-+24&vApfV zX+T5TzgKnt`B<^%=j#Y_-c5%8)}A{2(BR$s_s7e2%L0>)&goCzfIbqrR$5!FH~qB5 z{PT}XtoGjiXU@Orztvp7_rD&4{JZHTB%=OrKFF)hJ2`-5wb&o#<9&br_LOu2(+2~C Mr>mdKI;Vst0A#z6qyPW_ diff --git a/tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_2_large.png b/tests/backstop/reference/google-site-kit_Modules_Analytics4_Components_AudienceSegmentation_Dashboard_AudienceTile_Loading_0_document_2_large.png index b49bc67e50ad46c8b2138966f962b0b6c73d09c3..9acbdde09e61582ae786a0fc0eccff4978760ac9 100644 GIT binary patch literal 8217 zcmeAS@N?(olHy`uVBq!ia0y~yU`YXC4mO~ONy(w3K#H+A$lZxy-8q?;3=Hy@JzX3_ zD(1Yodp2*@Nim0uOvwT1O%p2qt8Yn(JmudkyU)7g&1~x(LD4O-e(AcMg|ZTPva7=U z{ryh^O=3{kJkJ(HADHhpfe}b|a2kQQhK?ZX6x2?DI7e6{fn>uB1!f>6APjU414B|1 z50G^5p{1tVXXGBFfB*hHdGpPluT{2TtFPW>2U_~Tc3NuX?-;%5A3uFEnm2!bdjHq7 zuV21w@sUl~Ds}s9S+2sE?Oc++JTzr%R42&v1MMGoSp>x|pEOO-5Tx`+ zM%-ebs5A zIpF_KRNClu>91PzKN;WKqktA(E~(o_)i( z?(2siCe>y7lNil5v+ua}al?FY$#-D)4O-TBlmr<# z**^T3`HcgZ0365a%i{#O2CIghkYGM_Y@3jp++4)TnEl|4DXYi2SKYDTM_{$+xFwj zZz_Z8hmD{F+_HMmmSre8ZV>fHu!Ss&^R~^jJ+QT}{VzDzG2A-}ZjaNpk3`T-*t&4w zWXs?0zu6s>9Fo8ZIDF8SWka^k$T6;N$d={*4as$Ty6MniD*LRkS?BcHeo(yrNbQd`pS z^YO9wPJeK(w_{)Z|9N_oQykMGr+s{Td;1gJe2_WyuZni|tdHNHcH1zy#WAf%KK{e) z?fEaoK`#7JKP~lS@)?Q336hVGb6t3Te*WKDPN4XVS5Kd&*8KZZd1`;9+0V|$?zO)@ zJw1IRZ+r6g+r7Z9dorls`+$Fo&S{Iee!=nzvxHLO=AM84@y{QdZMWY7)g1yUC@@~X z-eLav=P$3X^x-v1{(3Dg3WG2BazoVNY^(;`V=xIF&( zv+`R+{xbHN^KI?z>sQEw2Oz{gdDnuvo&QRPY*B@+SRkt`8P!DhR>x?Nj|TZbjmeIt zz0tHcn)b+@82PhY)>;hI#*lcDZ`&Q)3hJF7@Bt59pJ@RP-j8xd17tK&jOGP~(ei*{ b#D)RWg<#<+=F?|P22~ZFu6{1-oD!M<0kTEl literal 8032 zcmeAS@N?(olHy`uVBq!ia0y~yU`YXC4mO~ONy(w3K#H+A$lZxy-8q?;3=Hy8o-U3d z6?5L)J)L*kL%`vp+E=Gf|LwC5Y*h9VOBcK`Ig)3B!nLK7=P(zpdTSe*V|X z^bLF;SJ?DD{;07Jt07pOVv4TbglHV%%-<(XI_>FB>^QGj+pr7>n24`E3}($ zUOnBn>5Pf*R)9ts%E?>U9<8I!|H(R=VE~^{^*(rbQ^&^kkIbKn2MkX zK(471zdG&A-R7R+WFV*EzUM`wcXBD;e*fL;4&*cJnY}3W1~_JZoRiauH=6cF)7}8L&_*-i zXeJz9nQ(XYgvje(?j-G{!itmGrQI$M^3Bm2esjUq=p{$Shv@1{`sSzjzL3wkZF&~i5rtZ za(25u|C|$3Y3nAEa_q;q#dFj_U9TD!eSQ6xMxJ6R*Ggs2pFe*x_8w4_!Twd*?%w6g zm*-fg^%$J!`S_OI&fb2xy#P?^!T&X{XC0qm_(;L{*mltc$B(qNb(B=uI!Q zkvsmeLL?0+->|!KFKbogw6p!eH%)-n=1;YnfBtcSMbFJ$85S%Hinw?|8k(UN0$l^g>*Rx+BO zM)T8Xej05K4~~)B(b8zNG#V_W(VxqGvfQB7DbJ_v^Ny}N1Z=4%sDbA-(wsnT-%-wJ n7>uTa(Y(OGFd7EKFbo(^ul(~jT%cSPR0Vjt`njxgN@xNAp79?w From 2b21f2da09d53d904ec78cf507dc27c937c424ef Mon Sep 17 00:00:00 2001 From: Ben Bowler Date: Tue, 23 Jul 2024 08:48:08 +0100 Subject: [PATCH 4/4] Address NoAudienceBannerWidget render logic to show new loading state. --- .../__snapshots__/index.test.js.snap | 61 +++++++++++++++++++ .../dashboard/NoAudienceBannerWidget/index.js | 26 +++----- .../NoAudienceBannerWidget/index.test.js | 27 +------- assets/js/modules/analytics-4/index.js | 2 +- 4 files changed, 73 insertions(+), 43 deletions(-) diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/NoAudienceBannerWidget/__snapshots__/index.test.js.snap b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/NoAudienceBannerWidget/__snapshots__/index.test.js.snap index b3bfa719f8b..8718913bdd6 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/NoAudienceBannerWidget/__snapshots__/index.test.js.snap +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/NoAudienceBannerWidget/__snapshots__/index.test.js.snap @@ -1,5 +1,66 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`NoAudienceBannerWidget should render when there is no configured audience. 1`] = ` +
+
+
+
+
+
+ +
+
+

+ It looks like your visitor groups aren’t available anymore. + + . +

+

+ You can deactivate this widget in + + . +

+
+
+
+ +
+
+
+
+
+`; + exports[`NoAudienceBannerWidget should render with correct message when there are additional configurable audiences available 1`] = `
- Array.isArray( availableAudiences ) && - ! availableAudiences.includes( audience ) - ); - - const configurableAudiences = availableAudiences?.filter( - ( element ) => - Array.isArray( configuredAudiences ) && - ! configuredAudiences.includes( element ) + const hasNoMatchingAudience = configuredAudiences?.every( + ( audience ) => + Array.isArray( availableAudiences ) && + ! availableAudiences.includes( audience ) ); - - if ( hasNoMatchingAudience ) { + if ( + !! configuredAudiences && + ( configuredAudiences?.length === 0 || hasNoMatchingAudience ) + ) { return ( ); diff --git a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/NoAudienceBannerWidget/index.test.js b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/NoAudienceBannerWidget/index.test.js index 77b1331d79f..69ed8ad3a26 100644 --- a/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/NoAudienceBannerWidget/index.test.js +++ b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/NoAudienceBannerWidget/index.test.js @@ -61,29 +61,6 @@ describe( 'NoAudienceBannerWidget', () => { jest.clearAllMocks(); } ); - it( 'should not render when availableAudiences and configuredAudiences are not loaded', () => { - muteFetch( audienceSettingsRegExp ); - - const { container } = render( , { - registry, - } ); - - expect( container ).toBeEmptyDOMElement(); - } ); - - it( 'should not render when availableAudiences is not loaded', () => { - registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetAudienceSettings( { - configuredAudiences: [ 'properties/12345/audiences/1' ], - isAudienceSegmentationWidgetHidden: false, - } ); - - const { container } = render( , { - registry, - } ); - - expect( container ).toBeEmptyDOMElement(); - } ); - it( 'should not render when configuredAudiences is not loaded', () => { muteFetch( audienceSettingsRegExp ); @@ -115,7 +92,7 @@ describe( 'NoAudienceBannerWidget', () => { expect( container ).toBeEmptyDOMElement(); } ); - it( 'should not render when there is no configured audience.', () => { + it( 'should render when there is no configured audience.', () => { registry .dispatch( MODULES_ANALYTICS_4 ) .setAvailableAudiences( availableAudiences ); @@ -129,7 +106,7 @@ describe( 'NoAudienceBannerWidget', () => { registry, } ); - expect( container ).toBeEmptyDOMElement(); + expect( container ).toMatchSnapshot(); } ); it( 'should not render when configured audience is matching available audiences', () => { diff --git a/assets/js/modules/analytics-4/index.js b/assets/js/modules/analytics-4/index.js index a1280ec1817..54a8d5a6d8d 100644 --- a/assets/js/modules/analytics-4/index.js +++ b/assets/js/modules/analytics-4/index.js @@ -154,7 +154,7 @@ export const registerWidgets = ( widgets ) => { isActive: ( select ) => { const configuredAudiences = select( MODULES_ANALYTICS_4 ).getConfiguredAudiences(); - return configuredAudiences?.length > 0; + return !! configuredAudiences; }, }, [ AREA_MAIN_DASHBOARD_TRAFFIC_AUDIENCE_SEGMENTATION ]