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

Incorrect modal top position after scrolling on small screens #5358

Closed
eclarke1 opened this issue Jun 15, 2022 · 3 comments
Closed

Incorrect modal top position after scrolling on small screens #5358

eclarke1 opened this issue Jun 15, 2022 · 3 comments
Labels
P0 High priority Type: Bug Something isn't working

Comments

@eclarke1
Copy link
Collaborator

eclarke1 commented Jun 15, 2022

Bug Description

Bug bash issue: https://app.asana.com/0/1202258919887896/1202442610474584 please refer to Asana issue for background

The modal is incorrectly positioned on small screens after scrolling down the page. The reason is upon initial page load, there's the WP admin bar, which is not sticky, thus leaving the white space at the top.

image.png

Steps to reproduce

  1. Go to the main dashboard.
  2. Scroll down the page.
  3. Click on the dashboard sharing settings button.
  4. Notice how the modal does not take the whole screen.

Screenshots


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

Acceptance criteria

  • The dashboard sharing settings modal should always be 100% in width and height on small screens (< 600px).
  • The WordPress admin bar should be displayed only when the user has scrolled to the top of the page.

Implementation Brief

  • The incorrect positioning/extra spacing here is caused due to considering the spacing for the admin bar. Since the admin bar isn't displayed when scrolled down, we need to add additional styles to remove the extra spacing when scrolled down.
  • Using /assets/sass/components/global/_googlesitekit-dialog.scss, add styles to .googlesitekit-dialog wrapped with .admin-bar.googlesitekit-plugin--has-scrolled: top: 0; only for small devices, i.e. from tablet breakpoint, it should be set to top: 23px;, e.g.:
.admin-bar.googlesitekit-plugin--has-scrolled & {
    top: 0;

    @media (min-width: $bp-tablet) {
        top: 23px;
    }
}
  • Using /assets/sass/components/global/_googlesitekit-dialog.scss, add styles to .googlesitekit-dialog .mdc-dialog__surface wrapped with .admin-bar.googlesitekit-plugin--has-scrolled: height: 100vh; only for small devices, i.e. from tablet breakpoint, it should be set to height: auto;, e.g.:
.admin-bar.googlesitekit-plugin--has-scrolled & {
    height: 100vh;

    @media (min-width: $bp-tablet) {
        height: auto;
    }
}

Test Coverage

  • No new tests to be added.

QA Brief

  • In addition to using the Steps to Reproduce above, ensure the Dashboard Settings Dialog box is correctly rendered on mobile screens (width <600px) while scrolling a tiny bit, so that the WP Admin bar is still partially on the screen.
  • The Dialog box should also not have changed in position for all other screen sizes and should render normally.
  • While this issue does not change other dialog boxes directly, ensure that they are still rendered correctly. Examples of other dialog boxes: "Additional Permissions Required" error that appears when connecting new services/modules, the Disconnect Site Kit Dialog box when you click on your profile icon and click Disconnect, the Disconnect Module dialog box when disconnecting modules such as AdSense from the Settings page, etc.

Changelog entry

  • Fix Dashboard Sharing modal position on small screens.
@eclarke1 eclarke1 added P0 High priority Type: Bug Something isn't working labels Jun 15, 2022
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Jun 16, 2022
@eugene-manuilov eugene-manuilov self-assigned this Jun 16, 2022
@eugene-manuilov
Copy link
Collaborator

@asvinb do you mind reviewing this ticket?

@asvinb
Copy link
Collaborator

asvinb commented Jun 16, 2022

IB ✅

@asvinb asvinb removed their assignment Jun 16, 2022
@jimmymadon jimmymadon self-assigned this Jun 20, 2022
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Jun 23, 2022
@jimmymadon jimmymadon removed their assignment Jun 23, 2022
@asvinb asvinb assigned asvinb and jimmymadon and unassigned asvinb Jun 24, 2022
@jimmymadon jimmymadon assigned asvinb and unassigned jimmymadon Jun 24, 2022
@asvinb asvinb removed their assignment Jun 24, 2022
@techanvil techanvil assigned techanvil and unassigned techanvil Jun 27, 2022
@wpdarren wpdarren self-assigned this Jun 27, 2022
@wpdarren
Copy link
Collaborator

wpdarren commented Jun 27, 2022

QA Update: ✅

Verified:

  • The Dashboard Settings Dialog box is correctly rendered on mobile screens (width <600px) when the user slightly scrolls , so that the WP Admin bar is still partially on the screen.
  • The Dialog box has not have changed in position for all other screen sizes and they render normally.
  • Other dialog boxes are rendered correctly as per the QAB.
Screenshots

image
image
image
image

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

8 participants