google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.23k stars 279 forks source link

Minor UX/UI Issue on Disconnect RRM Module #9061

Open wpdarren opened 1 month ago

wpdarren commented 1 month ago

Bug Description

While testing #8845 I noticed that the engineering does not match the figma design. There is a styling issue where the bullet point is not aligned to the text like the figma designs and in addition the horizontal line has no space between the text underneath. Only a minor UX/UI issue but it looks odd.

image.png

Steps to reproduce

  1. Set up RRM module
  2. Go to Site Kit Settings > Connected modules > and Edit RRM
  3. Click on the link to disconnect the module and the modal will appear
  4. See UX/UI issue

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Follow the steps to reproduce the issue and match the alignment of bullet to that of the figma design.

Changelog entry

nfmohit commented 2 weeks ago

IB ✅

kelvinballoo commented 1 week ago

QA Update ⚠️

This is verified good overall. My only flag is a potential regression on other modals. ⚠️

I did a test on Ads module. On the modal, there will be 3 bullets and there will be 2 horizontal lines separating them all. The last bullet will not have a horizontal line below it. But when I cross-check with figma, it would eventually have a horizontal line for each bullet. I was wondering if this is intentional or this is a regression.

Implementation: Screenshot 2024-09-04 at 18 28 13 Figma: Screenshot 2024-09-04 at 18 37 33

Other than that, those were verified good:

ankitrox commented 1 week ago

@kelvinballoo This is intentional and we have removed the bottom border from last item to maintain the consistency with RRM disconnect dialog.

CC: @benbowler @techanvil

kelvinballoo commented 1 week ago

QA Update ✅

Thanks for confirming @ankitrox . This is good to be moved to approval.