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
Enhance/#8177 - Add the settings section toggle switch for audience segmentation widget area visibility #8424
Conversation
Build files for 5f47018 have been deleted. |
Size Change: +291 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
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.
<Cell size={ 12 }> | ||
<Switch | ||
label={ __( | ||
'Display visitors groups in dashboard', |
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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @aaemnnosttv, I've created #8496 to address this.
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 👍
Summary
Addresses issue:
Relevant technical choices
assets/js/modules/analytics-4/components/settings/AudienceSegmentation
directory. However, as a convention, all other admin settings files and components are inassets/js/components/settings/
. Hence, theSettingsVisitorGroups
component is created inassets/js/components/settings/
.PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist