google / google/site-kit-wp

Email subscription panel list not scrollable, background scrolls instead"

Open
#13,499 0 comments 0 reactions 0 assignees View on GitHub
P1 Type: Bug
Dominant language
JavaScript
Stars
1.4k
Forks
383
Avg merge
4d 14h
Merged PRs (30d)
77

Description

## Bug Description

This came up while testing the email subscription panel.
I had an issue on the mobile version. A video is attached for reference:
- At first glance, the list of the subscribed user doesn't show and it's only the filter input field.
- That's probably fine but when I tried to scroll on the panel, the background would scroll and not the panel items. Noting that it was not not at the subscribed list area.
- If we the panel is open, I don't think we should scroll the background. Might be better to scroll the panel so that we can see the list of users.
Not exactly sure the best way to improve this but that's my though.

https://github.com/user-attachments/assets/1f222ca7-f963-4e98-8b9a-f877552e19fe

## Steps to reproduce

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

## Screenshots

## Additional Context

- PHP Version:
- OS: [e.g. iOS]
- Browser: [e.g. chrome, safari]
- Plugin Version: [e.g. 22]
- Device: [e.g. iPhone6]

---------------

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

## Acceptance criteria

- When the Email Reports user settings panel is open and the "Subscribed users" or "Invite others to subscribe" tab is selected:
- The user list is always visible with a usable height, regardless of the viewport height. It must not collapse so that only the search input is shown.
- On viewports where the panel content doesn't fit, the panel content scrolls, so the tabs, search input and user list can be reached.
- On viewports with enough space, the current behaviour stays the same: the user list fills the remaining panel height and scrolls within itself.
- While any side sheet panel is open, scrolling inside the panel must not scroll the page behind it.

## Implementation Brief

- Update `assets/sass/components/email-reporting/_googlesitekit-subscriber-management.scss`
- Remove `min-height: 0` https://github.com/google/site-kit-wp/blob/4e5ab8c633abb55b23e35d1556cbbe1c22db11e0/assets/sass/components/email-reporting/_googlesitekit-subscriber-management.scss#L25-L43 from `.googlesitekit-subscriber-management` and `.googlesitekit-subscribed-users`.
- Note: these wrappers are flex items inside the scrollable `.googlesitekit-selection-panel__content`. With `flex-shrink: 1` and `min-height: 0` they shrink to fit the leftover space instead of making the panel content overflow. On short viewports that leaves 0px for the user list.
- Update `assets/sass/components/email-reporting/_googlesitekit-invite-others-to-subscribe.scss`
- Make the same change for `.googlesitekit-invite-others-to-subscribe`.
- Update `.googlesitekit-user-list` in `_googlesitekit-user-list.scss`
- Use a `0` flex-basis (`flex: 1 1 0`). The list's full content height then no longer counts toward the wrappers' minimum size, so the list still fills the remaining space and scrolls internally on taller viewports.
- Replace `min-height: 0` with a minimum height that fits about 3 user rows, matching `UserListSkeleton` with `visibleItems={ 3 }`. On short viewports the list keeps this height and the panel content becomes scrollable.
- Add `overscroll-behavior: contain`, so reaching the end of the list doesn't pass the scroll on.
- Update `.googlesitekit-selection-panel > .googlesitekit-selection-panel__content` in `_googlesitekit-selection-panel.scss`
- Add `overscroll-behavior: contain`, so scrolling doesn't pass from the panel content to the page.
- Harden the side sheet scroll lock, which currently only sets `overflow: hidden` on `body` https://github.com/google/site-kit-wp/blob/4e5ab8c633abb55b23e35d1556cbbe1c22db11e0/assets/sass/components/global/_googlesitekit-side-sheet.scss#L55-L57. iOS Safari doesn't reliably honor that for touch scrolling.
- In `SideSheet`, toggle the existing `googlesitekit-side-sheet-scroll-lock` class on `document.documentElement` as well, next to the existing `body` toggle https://github.com/google/site-kit-wp/blob/4e5ab8c633abb55b23e35d1556cbbe1c22db11e0/assets/js/components/SideSheet.js#L75-L88.
- Extend the scroll lock rule so it applies to both `html` and `body`, and add `overscroll-behavior: none` there.
- Note: `SideSheet` is shared, so this also fixes background scrolling for the other side panels (Key Metrics, Audiences, etc.).
- Check the result on a real iOS Safari device, or BrowserStack, at a few viewport heights, on both tabs.

### Test Coverage

- Update `assets/js/components/SideSheet.test.js`: extend the "should lock document scroll when opened" case to assert the scroll lock class is also added to `document.documentElement`, and removed from both elements when the side sheet is closed.
- No other Jest, PHPUnit or E2E changes expected. The layout fix is CSS only.
- VRT: no changes expected for the existing `UserSettingsSelectionPanel` and `SubscribedUsers` scenarios. They render at desktop height, where the list behaviour doesn't change. If any reference image changes, review it and approve it if the change is intended.

## QA Brief

*

## Changelog entry

*

Contributor guide

Open the contributing guide

Research direction

Start with the listed subscriber-management, invite-others, user-list, and selection-panel Sass files, then inspect the scroll-lock logic in assets/js/components/SideSheet.js. Run assets/js/components/SideSheet.test.js and verify both tabs at several viewport heights, including iOS Safari or BrowserStack. Done means the list remains usable, panel content scrolls when needed, and the page behind any open side sheet does not scroll.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, sass
Domain
frontend, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.