-
Notifications
You must be signed in to change notification settings - Fork 286
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
Update design and layout of Analytics settings edit view #5151
Comments
I agree, but I think UX is working on redesigning the layout there anyways. I think styling the heading/labels differently would be a big improvement, not just for this one instance.
The tricky thing here is that an additional control is revealed if this is toggled off so it makes more sense to have them as they are now. We probably just need better separation between groups of settings. |
here is the figma file https://www.figma.com/file/f1I9Mc0ITE6Uk0qiTRbggK/GM2%2B?node-id=1011%3A261 |
Thanks @sebastianmantel ! This seems like a simple enough change (add some more separation lines and clear headings. @aaemnnosttv what are your thoughts on this approach to addressing this, should be pretty low effort, right? |
@marrrmarrr yes this should be relatively low effort I think (assuming I didn't just jinx it). Like @sebastianmantel said, I think we just need to solidify the names of the headings that we didn't have before. I just took a quick look at the other modules as well and none of them seem to have the same problem so we shouldn't need to make changes to any of the others 👍 |
IB ✅ |
QA Update:
|
@wpdarren the purpose of this issue was to create a better visual structure on the settings screen. Primarily by introducing new headlines between the individual settings, but no larger styling changes. The AC mentioned:
Because of the AC is mentioning, some of the styles are inconsistent, I suspect that's exactly what you are experiencing. Regarding your second question, I actually don't know why the text is different, as renaming settings ui wasn't part of this issue. Maybe thats also one of the inconsistencies mentioned in the AC? |
Feature Description
This a tiny nitpicky thing, but I'm working on some documentation related to the Exclude from Analytics setting at Settings > Site Kit > Connected Services > Analytics and I find the UX kind of confusing:
The other toggle-based settings on this screen follow a format of toggle + description. This one has a "title" (Exclude from Analytics), then a toggle + description. It makes it kind of hard to parse, especially since the title is not bolded or set off in any way.
My thinking would be remove the title and change the toggle text to "Exclude logged-in users from Analytics".
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
assets/js/modules/analytics/components/settings/GA4SettingsControls.js
assets/js/modules/analytics/components/common/TrackingExclusionSwitches.js
assets/js/modules/analytics/components/common/AdsConversionIDTextField.js
assets/js/modules/analytics/components/common/AnonymizeIPSwitch.js
div
having thegooglesitekit-settings-module__fields-group
if not already so.h4
having thegooglesitekit-settings-module__fields-group-title
class name with the strings set as per the designs in Figma.assets/sass/components/settings/_googlesitekit-settings-module.scss
,googlesitekit-settings-module__fields-group-title
class name as per the designs.Test Coverage
QA Brief
Changelog entry
The text was updated successfully, but these errors were encountered: