Skip to content
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

"Edit in Analytics" and "Edit in Tag Manager" settings links lack visual whitespace due to HTML markup #7968

Closed
2 of 3 tasks
tofumatt opened this issue Dec 8, 2023 · 13 comments
Labels
P0 High priority Type: Bug Something isn't working

Comments

@tofumatt
Copy link
Collaborator

tofumatt commented Dec 8, 2023

Bug Description

The "Edit in" links for Analytics recently had their display changed to inline-flex, preventing the whitespace from rendering properly, leading to the text looking like "Editin Analytics".

The same is likely true for Tag Manager, as the HTML markup is the same.

Steps to reproduce

  1. Set up Site Kit with Analytics
  2. Go to Site Kit Settings
  3. View Analytics
  4. See error

Screenshots

CleanShot 2023-12-08 at 16 50 52


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

Acceptance criteria

  • The "Edit in Analytics" and "Edit in Tag Manager" links should have visual whitespace between each word.

Implementation Brief

The issue will apply to any link with <VisuallyHidden> content, as the recent change to display: inline-flex; causes the whitespace around the <VisuallyHidden /> component to be "squashed".

  • Change .googlesitekit-cta-link from display: inline-flex; to display: inline-block;
  • Add vertical-align: text-bottom; to .googlesitekit-cta-link .googlesitekit-icon-wrapper
  • Merge Fix whitespace appearance in CTA links with icons. #7969 (PR is ready and can be moved directly to Code Review after IB Review)

Test Coverage

  • VRTs may need updating, but no new tests are needed.

QA Brief

  • Visit Analytics settings and Tag Manager settings in Site Kit.
  • The "Edit in Analytics" and "Edit in Tag Manager" should not appear to have "Editin" as one word.

Changelog entry

  • Fix whitespace issue in Analytics and Tag Manager settings.
@aaemnnosttv
Copy link
Collaborator

Change .googlesitekit-cta-link from display: inline-flex; to display: inline-block;

👍

@tofumatt I think we can avoid the need for adding the top positioning by using vertical-align (middle or maybe text-bottom) on .googlesitekit-cta-link .googlesitekit-icon-wrapper. WDYT?

image

@aaemnnosttv aaemnnosttv assigned tofumatt and unassigned aaemnnosttv Dec 12, 2023
@tofumatt
Copy link
Collaborator Author

@aaemnnosttv Nice, yeah, looks like that does the trick 👍🏻

@tofumatt tofumatt assigned aaemnnosttv and unassigned tofumatt Dec 12, 2023
@aaemnnosttv
Copy link
Collaborator

IB ✅ moving to CR

@aaemnnosttv aaemnnosttv assigned tofumatt and unassigned aaemnnosttv Dec 13, 2023
@tofumatt tofumatt removed their assignment Dec 13, 2023
@tofumatt tofumatt assigned aaemnnosttv and unassigned tofumatt Dec 13, 2023
@aaemnnosttv aaemnnosttv removed their assignment Dec 13, 2023
@wpdarren wpdarren self-assigned this Dec 13, 2023
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • The "Edit in Analytics" and "Edit in Tag Manager" do not have "Editin" as one word in settings.
Screenshots

image
image

@wpdarren wpdarren removed their assignment Dec 14, 2023
@wpdarren
Copy link
Collaborator

QA Update: ❌

@tofumatt I have just noticed that the link text and icon aren't aligned with the text. It's only slightly off but noticeable.

image
image

@tofumatt tofumatt assigned aaemnnosttv and unassigned tofumatt Dec 14, 2023
@aaemnnosttv aaemnnosttv assigned wpdarren and unassigned aaemnnosttv Dec 15, 2023
@aaemnnosttv
Copy link
Collaborator

@wpdarren ready for another look 👍

@aaemnnosttv
Copy link
Collaborator

Actually it still has at least one more thing to fix
image

@aaemnnosttv aaemnnosttv assigned tofumatt and unassigned wpdarren Dec 15, 2023
@wpdarren
Copy link
Collaborator

Mmm, yes, those Edit links were working before, so it seems a regression.

@tofumatt tofumatt mentioned this issue Dec 15, 2023
18 tasks
@mohitwp
Copy link
Collaborator

mohitwp commented Dec 15, 2023

@tofumatt

Please found all issues details below with description and screenshots cc @wpdarren @kuasha420 @bethanylang @aaemnnosttv @eclarke1

1) Main dashboard and entity dashboard > All Traffic widget> text moved to next line.

image

2) Key metrics widget > 'Change metrics' button SVG is not aligned properly with text on main.

image

3) User Input Customize site goals screen and under settings - No Space between icon and edit label.

image

image

4) Key metrics :Ellipsis are missing on main environment for KM "Most engaging pages" widget and similar widgets.

image

image

5) The icon against the URL on the entity dashboard is misaligned again. This was fixed previously.

image

6) Source CTA svg icons are not looking bold now on main. I noticed that for svg icons we have different is getting apply on main which is not the case on latest. This same issue exist wherver we are using this icon like under notifications, Settings page >Connected services, Admin Settings> tracking services, main and entity dashboard, Settings> Connected Services> Analytics.

image

image

image

image

image

image

image

image

7) Connected Services > Arrows under green circle are not center aligned on main.

image

image

8) Connected services> Disconnect buttons for all modules are not aligned properly.

image

image

9) Settings> Connected Services> 'Edit' icons for all modules not aligned with the text.

image

image

10) Settings> Connect More Services > Set up arrow is not aligned with the text.

image

11) The Edit links for Analytics and Tag Manager settings are slightly misaligned.

image

@tofumatt
Copy link
Collaborator Author

tofumatt commented Dec 15, 2023

For context, the change to display: inline-flex; for .googlesitekit-cta-link has had a lot of knock-on effects throughout the plugin. For instance, the ellipsis bug mentioned in 4 caused the link to have a width wider than the widget, and thus never overflow.

I'm going to mark which of these are totally fixed and which are either issues for a follow-up, acceptable changes, etc. here. (Currently a WIP, don't reply yet 😄)

  • 1. Main dashboard and entity dashboard > All Traffic widget> text moved to next line.
  • 2. Key metrics widget > 'Change metrics' button SVG is not aligned properly with text on main.
  • 3. User Input Customize site goals screen and under settings - No Space between icon and edit label.
  • 4. Key metrics :Ellipsis are missing on main environment for KM "Most engaging pages" widget and similar widgets.
  • 5. The icon against the URL on the entity dashboard is misaligned again. This was fixed previously.
  • 6. Source CTA svg icons are not looking bold now on main. This is correct and an intended effect.
  • 7. Connected Services > Arrows under green circle are not center aligned on main.
  • 8. Connected services> Disconnect buttons for all modules are not aligned properly.
  • 9. Settings> Connected Services> 'Edit' icons for all modules not aligned with the text.
  • 10. Settings> Connect More Services > Set up arrow is not aligned with the text.
  • 11. The Edit links for Analytics and Tag Manager settings are slightly misaligned.

@tofumatt tofumatt mentioned this issue Dec 15, 2023
18 tasks
@tofumatt tofumatt assigned aaemnnosttv and unassigned tofumatt Dec 15, 2023
@aaemnnosttv
Copy link
Collaborator

This should be good now @wpdarren @mohitwp. @tofumatt and I went through everything pretty thoroughly. Back to you guys 👍

@aaemnnosttv aaemnnosttv assigned wpdarren and mohitwp and unassigned aaemnnosttv Dec 15, 2023
@wpdarren
Copy link
Collaborator

QA Update: ⚠️

@mohitwp would appreciate a second pair of eyes on this, but the only issue I can find is that the Edit in Analytics link highlighted below is slightly off. Everything else seems fixed, and I can't see any regressions.

image

Verified:

  1. Main dashboard and entity dashboard > All Traffic widget> text fixed with alignment of chart legends.
  2. Key metrics widget > 'Change metrics' button SVG is aligned properly with text.
  3. User Input Customize site goals screen and under settings - Space between icon and edit label is fixed.
  4. Key metrics :Ellipsis are appearing for KM "Most engaging pages" widget and similar widgets.
  5. The icon against the URL on the entity dashboard is aligned correctly.
  6. Connected Services > Arrows under green circle are center aligned.
  7. Connected services> Disconnect buttons for all modules are aligned properly.
  8. Settings> Connected Services> 'Edit' icons for all modules are aligned with the text.
  9. Settings> Connect More Services > Set up arrow is aligned with the text.
  10. The Edit links for Analytics and Tag Manager settings are misaligned. See note above.
Screenshots

image
image
image
image
image
image
image
image
image
image
image
image
image

@wpdarren wpdarren removed their assignment Dec 18, 2023
@mohitwp
Copy link
Collaborator

mohitwp commented Dec 18, 2023

QA Update ✅

@wpdarren Tested all the points and all issues are fixed.

The issue you highlighted is basically a part of point 6 because we are using same icons here and now these icons are not bold on main environment after new changes. For 6 @tofumatt replied that it is intended. Other issues are fixed now.

  1. Source CTA svg icons are not looking bold now on main. This is correct and an intended effect.

@mohitwp mohitwp removed their assignment Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants