-
Notifications
You must be signed in to change notification settings - Fork 284
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
UX/UI issues discovered on Thank with Google module #5689
Labels
Module: Thank with Google
Thank with Google module related issues
P1
Medium priority
Type: Bug
Something isn't working
Comments
wpdarren
added
Type: Bug
Something isn't working
Module: Thank with Google
Thank with Google module related issues
labels
Aug 15, 2022
Added P1 label as a nice to have for bug bash. |
Thanks @wpdarren Regarding # 1 – this is intentional and part of the design for the "Pending Approval" step in Figma. As for # 4 – is this still a problem with other browsers for the other elements you mentioned or we were able to fix it? It's not ideal but I feel like this should be doable in Safari as well. Definitely not a launch blocker 😄 |
IB ✔️ |
QA Update: ✅Verified:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Module: Thank with Google
Thank with Google module related issues
P1
Medium priority
Type: Bug
Something isn't working
Bug Description
While running through my test plan for Thank with Google, I noticed a number of styling / missing images from the module that were not appearing when we initially completed the QA. These will be brought up at the bug bash so ideally, it would be good to get these fixed and not have them as known issues.
Non-issues
#161B18
rather than#3C7251
Issue 2 and 3 are part of the fix in #5627
The images on the splash pages are no longer appearing as you can see from the screenshot above. The screenshot below is what is appearing on Figma designs and were there previously. I am using 1.90.0 of the tester plugin but I know we have a new version so it could be possibly linked but unsure.
The images within the image radio buttons are no longer appearing when customizing TwG. Also the size of the two buttons are no longer the same height.
When using Safari, the colour border is square instead of round like on Chrome and other browsers. We've had this issue with other elements of Site Kit (i.e. chip navigation on dashboard), but thought it was worth highlighting.
Extra padding in setup and settings
Screenshots
Setup with extra padding
Settings view with extra padding
Settings edit with correct padding
Example setup with correct padding
Minor layout shift when selected Prominence choice changes
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
The border radius issue in safari is caused by the browser not applying border radius styles to
outline
-properties in CSS. This can be remedied by refactoring the outline to use a pseudo element instead.In
assets/sass/components/global/_googlesitekit-image-radio.scss
:::after
pseudoelement to the.mdc-image-radio__background
selector.mdc-image-radio__content
.mdc-image-radio__content
selector and add it to the::after
element as an border::after
elementIn
assets/sass/components/thank-with-google/_googlesitekit-twg-color-radio.scss
:The layout shifting is caused by the outline getting thicker in the checked state, and the border getting thinner to compensate for it.
When refactoring the outline to a pseudo-element, ensure that the combined width of the white border on
mdc-image-radio__content
and the green/gray border on.mdc-image-radio__background::after
is the same for both the checked and unchecked variants.The extra padding is caused by the use of nested grid elements in the module, both adding 24px of padding inside of the module.
In
assets/js/modules/thank-with-google/components/settings/SettingsView.js
:<Grid>...</Grid>
to closer match the implementation of theSettingsView
component in other modules (e.g:assets/js/modules/tagmanager/components/settings/SettingsView.js
,assets/js/modules/analytics/components/settings/SettingsView.js
)Test Coverage
QA Brief
Changelog entry
The text was updated successfully, but these errors were encountered: