-
Notifications
You must be signed in to change notification settings - Fork 277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhance/#8177 - Add the settings section toggle switch for audience segmentation widget area visibility #8424
Changes from all commits
69b1803
4f3808b
45207ef
64c9ddc
5f47018
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
/** | ||
* SettingsCardVisitorGroups 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. | ||
*/ | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { useCallback } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import Data from 'googlesitekit-data'; | ||
import { Switch } from 'googlesitekit-components'; | ||
import { Cell, Grid, Row } from '../../material-components'; | ||
import Layout from '../layout/Layout'; | ||
import { MODULES_ANALYTICS_4 } from '../../modules/analytics-4/datastore/constants'; | ||
const { useDispatch, useSelect } = Data; | ||
|
||
export default function SettingsCardVisitorGroups() { | ||
const audienceSegmentationWidgetHidden = useSelect( ( select ) => | ||
select( MODULES_ANALYTICS_4 ).isAudienceSegmentationWidgetHidden() | ||
); | ||
|
||
const { setAudienceSegmentationWidgetHidden, saveAudienceSettings } = | ||
useDispatch( MODULES_ANALYTICS_4 ); | ||
|
||
const handleKeyMetricsToggle = useCallback( async () => { | ||
await setAudienceSegmentationWidgetHidden( | ||
! audienceSegmentationWidgetHidden | ||
); | ||
await saveAudienceSettings(); | ||
}, [ | ||
audienceSegmentationWidgetHidden, | ||
saveAudienceSettings, | ||
setAudienceSegmentationWidgetHidden, | ||
] ); | ||
|
||
return ( | ||
<Layout | ||
className="googlesitekit-settings-meta" | ||
title={ __( 'Visitor groups', 'google-site-kit' ) } | ||
header | ||
fill | ||
rounded | ||
> | ||
<div className="googlesitekit-settings-module googlesitekit-settings-module--active"> | ||
<Grid> | ||
<Row> | ||
<Cell size={ 12 }> | ||
<Switch | ||
label={ __( | ||
'Display visitors groups in dashboard', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a misspelling which was copied from the design. I've raised it there as well but we should fix this in a follow-up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @aaemnnosttv, I've created #8496 to address this. |
||
'google-site-kit' | ||
) } | ||
checked={ ! audienceSegmentationWidgetHidden } | ||
onClick={ handleKeyMetricsToggle } | ||
hideLabel={ false } | ||
/> | ||
</Cell> | ||
</Row> | ||
</Grid> | ||
</div> | ||
</Layout> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
/** | ||
* SettingsCardVisitorGroups component stories. | ||
* | ||
* 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. | ||
*/ | ||
|
||
/** | ||
* External dependencies | ||
*/ | ||
import fetchMock from 'fetch-mock'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { MODULES_ANALYTICS_4 } from '../../modules/analytics-4/datastore/constants'; | ||
import WithRegistrySetup from '../../../../tests/js/WithRegistrySetup'; | ||
import SettingsCardVisitorGroups from './SettingsCardVisitorGroups'; | ||
|
||
function Template() { | ||
return <SettingsCardVisitorGroups />; | ||
} | ||
|
||
export const Default = Template.bind( {} ); | ||
Default.storyName = 'Default'; | ||
Default.scenario = { | ||
label: 'Modules/Analytics4/Components/AudienceSegmentation//SettingsCardVisitorGroups/Default', | ||
}; | ||
|
||
export default { | ||
title: 'Modules/Analytics4/Components/AudienceSegmentation/SettingsCardVisitorGroups', | ||
decorators: [ | ||
( Story, { args } ) => { | ||
const setupRegistry = ( registry ) => { | ||
registry | ||
.dispatch( MODULES_ANALYTICS_4 ) | ||
.receiveGetAudienceSettings( { | ||
configuredAudiences: [ 'audienceA', 'audienceB' ], | ||
isAudienceSegmentationWidgetHidden: false, | ||
} ); | ||
|
||
// Mock the audience-settings endpoint to allow toggling the switch. | ||
fetchMock.post( | ||
RegExp( | ||
'google-site-kit/v1/modules/analytics-4/data/audience-settings' | ||
), | ||
( url, { body } ) => { | ||
const { data } = JSON.parse( body ); | ||
|
||
return { body: data.settings }; | ||
} | ||
); | ||
|
||
if ( args.setupRegistry ) { | ||
args.setupRegistry( registry ); | ||
} | ||
}; | ||
|
||
return ( | ||
<WithRegistrySetup func={ setupRegistry }> | ||
<Story /> | ||
</WithRegistrySetup> | ||
); | ||
}, | ||
], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
/** | ||
* SettingsCardVisitorGroups component tests. | ||
* | ||
* 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. | ||
*/ | ||
|
||
import { render, waitFor } from '../../../../tests/js/test-utils'; | ||
import { createTestRegistry } from '../../../../tests/js/utils'; | ||
import { MODULES_ANALYTICS_4 } from '../../modules/analytics-4/datastore/constants'; | ||
import SettingsCardVisitorGroups from './SettingsCardVisitorGroups'; | ||
|
||
describe( 'SettingsCardVisitorGroups', () => { | ||
let registry; | ||
|
||
beforeEach( () => { | ||
registry = createTestRegistry(); | ||
} ); | ||
|
||
it( 'should render the visitor groups switch correctly', async () => { | ||
registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetAudienceSettings( { | ||
configuredAudiences: [ 'audienceA', 'audienceB' ], | ||
isAudienceSegmentationWidgetHidden: false, | ||
} ); | ||
|
||
const { getByLabelText } = render( <SettingsCardVisitorGroups />, { | ||
registry, | ||
} ); | ||
|
||
const switchControl = getByLabelText( | ||
'Display visitors groups in dashboard' | ||
); | ||
|
||
await waitFor( () => { | ||
expect( switchControl ).toBeChecked(); | ||
} ); | ||
} ); | ||
|
||
it( 'should toggle the switch on click and save the audience settings', async () => { | ||
const audienceSettingsEndpoint = new RegExp( | ||
'^/google-site-kit/v1/modules/analytics-4/data/audience-settings' | ||
); | ||
|
||
registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetAudienceSettings( { | ||
configuredAudiences: [ 'audienceA', 'audienceB' ], | ||
isAudienceSegmentationWidgetHidden: true, | ||
} ); | ||
|
||
fetchMock.postOnce( audienceSettingsEndpoint, { | ||
body: { | ||
configuredAudiences: [ 'audienceA', 'audienceB' ], | ||
isAudienceSegmentationWidgetHidden: false, | ||
}, | ||
status: 200, | ||
} ); | ||
|
||
const { getByLabelText } = render( <SettingsCardVisitorGroups />, { | ||
registry, | ||
} ); | ||
|
||
const switchControl = getByLabelText( | ||
'Display visitors groups in dashboard' | ||
); | ||
|
||
expect( switchControl ).not.toBeChecked(); | ||
|
||
switchControl.click(); | ||
|
||
await waitFor( () => { | ||
expect( switchControl ).toBeChecked(); | ||
|
||
// Ensure the proper body parameters were sent. | ||
expect( fetchMock ).toHaveFetched( audienceSettingsEndpoint, { | ||
body: { | ||
data: { | ||
settings: { | ||
configuredAudiences: [ 'audienceA', 'audienceB' ], | ||
isAudienceSegmentationWidgetHidden: false, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
} ); | ||
} ); | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @hussain-t, just a heads up that while I did spot your note about the location for this file in relevant technical choices, the location specced in the IB was actually correct, in line with the code organization section of the design doc and as mentioned here: #8177 (comment)
An extra wrinkle is that in fact we have also identified this initial code org as needing improvement and we're going to move the Audience Segmentation component files under
assets/js/modules/analytics-4/components/audience-segmentation/
instead.I've created #8495 to address the wider reorganisation, but in the meantime we should stick to the code structure outlined in the design doc to make it easy to keep track of all the Audience Segmentation components and move them appropriately when the time comes.
I've also created #8496 to address the typo that Evan spotted and mentioned separately in this issue (which of course was simply copied from Figma and not introduced via this PR), and to move this component at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing it out, @techanvil 👍