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

Tooltip overlapping the header #5938

Closed
asvinb opened this issue Sep 28, 2022 · 6 comments
Closed

Tooltip overlapping the header #5938

asvinb opened this issue Sep 28, 2022 · 6 comments
Labels
P2 Low priority Type: Enhancement Improvement of an existing feature

Comments

@asvinb
Copy link
Collaborator

asvinb commented Sep 28, 2022

Bug Description

Currently tooltips have a very high z-index (20000) which results in them being on top of other elements on the page. However we have an issue with sticky elements where there is an overlap. This is the case with the header where on small screens, upon scrolling the page, the tooltip overlaps the header.

Steps to reproduce

  1. Go to the Analytics module setup screen on a mobile device/small viewport.
  2. Click on 'Connect Google Analytics 4'
  3. The tooltip is displayed.
  4. Scroll up the page
  5. Notice how the tooltip overlaps the header.

Screenshots

image


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

Acceptance criteria

  • The tooltip to setup a GA4 property only should not overlap the header upon scrolling on small screens.

Implementation Brief

  • Using assets/js/modules/analytics/components/settings/GA4SettingsControls.js,
    • Update JoyrideTooltip to include the styles prop with the following value:
    {
        options: {
            zIndex: 10,
        },
    }

Test Coverage

  • No new tests to be added.

QA Brief

  • Follow Steps to reproduce - tooltip should be fixed now as per screenshot:

Screenshot 2023-01-17 at 12 14 01

Changelog entry

  • Prevent the "Set up Google Analytics 4..." Tooltip from overlapping the header on page scroll.
@aaemnnosttv
Copy link
Collaborator

@asvinb The tooltip you have shown is really a one-off feature tour (without the backdrop) which should be on a higher plane than pretty much everything else. If we want a different behavior in this case, we should maybe have a more "inline" version of this or perhaps add a class to this instance specifically which reduces its z-index rather than lowering it for all which would break tours that highlight elements of the header/navigation, for example.

@aaemnnosttv aaemnnosttv added P2 Low priority Type: Enhancement Improvement of an existing feature labels Dec 16, 2022
@asvinb
Copy link
Collaborator Author

asvinb commented Dec 19, 2022

@aaemnnosttv Agree with not lowering the tooltip z-index globally but instead changing the z-index only to be lower than the header z-index only for this tour.

@asvinb asvinb removed their assignment Dec 19, 2022
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Jan 4, 2023
@tofumatt
Copy link
Collaborator

tofumatt commented Jan 4, 2023

ACs are good 👍🏻

@asvinb asvinb assigned asvinb and unassigned asvinb Jan 6, 2023
@eugene-manuilov eugene-manuilov self-assigned this Jan 9, 2023
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Jan 9, 2023
@sashadoes sashadoes self-assigned this Jan 14, 2023
@sashadoes sashadoes removed their assignment Jan 17, 2023
@techanvil techanvil assigned techanvil and unassigned techanvil Jan 17, 2023
@wpdarren wpdarren self-assigned this Jan 18, 2023
@wpdarren
Copy link
Collaborator

wpdarren commented Jan 18, 2023

QA Update: ❌

@sashadoes I have an observation that I'd like to run by you.

All looks good on small screen sizes like mobile and tablet, but on desktop when I scroll down, the tooltip shifts placement and now it doesn't overlap the WordPress side menu, and looks a little odd.

This is 1.92.0 the latest release
image

This is how I see it now on the develop branch
image

Interested in your thoughts here, but in my opinion, I do think it should overlap on the side menu.

@wpdarren wpdarren assigned sashadoes and unassigned wpdarren Jan 18, 2023
@sashadoes sashadoes mentioned this issue Jan 18, 2023
18 tasks
@sashadoes sashadoes removed their assignment Jan 18, 2023
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • On small screen sizes, the tooltip to setup a GA4 property does not overlap the header upon scrolling.
  • I also tested this on desktop to ensure that the behaviour is the same as currently. Found an issue which is now fixed.
  • Tested on various small screen devices like mobile and tablet on our supported browsers.

Note: I did discover a styling issue on small screen sizes related to the tooltip but it wasn't a regression related to this ticket, so will create a separate ticket for it.

Screenshots

image
image

@wpdarren wpdarren removed their assignment Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Low priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants