From fb10ec77e46ed6e4a5d965254eb56efd654c048a Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 7 May 2026 17:27:53 +0100 Subject: [PATCH 1/3] fix(7696): make settings popup scroll on short viewports Move max-height + overflow:auto out of the mobile-only media query and onto the base .popup-content rule so the Settings popup (and other popups) gain a scrollbar instead of cropping items off-screen when the window is short. Pad-wide Settings is the worst offender because it adds a second column of controls plus a Delete pad button. Adds a Playwright regression test that verifies the popup is scrollable and the Delete pad button is reachable at a 900x500 viewport. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/static/css/pad/popup.css | 15 ++++++++---- .../frontend-new/specs/pad_settings.spec.ts | 23 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/static/css/pad/popup.css b/src/static/css/pad/popup.css index a0d989c147b..2035774d6af 100644 --- a/src/static/css/pad/popup.css +++ b/src/static/css/pad/popup.css @@ -30,6 +30,17 @@ background: #f7f7f7; min-width: min(300px, 90vw); max-width: min(600px, 95vw); + /* Constrain height so popups (notably Settings with Pad-wide Settings + enabled) scroll instead of cropping items off-screen on short windows. + Fixes #7696. */ + max-height: calc(100vh - 20px); + overflow-y: auto; +} + +/* Chat manages its own scroll and floats author-colour pickers outside the + popup, so it must not become a scroll container. */ +.popup#users .popup-content { + overflow: visible; } .popup input[type=text] { width: 100%; @@ -76,10 +87,6 @@ } .popup-content { max-height: 80vh; - overflow: auto; - } - .popup#users .popup-content { - overflow: visible; } } /* Move popup to the bottom, except popup linked to left toolbar, like hyperklink popup */ diff --git a/src/tests/frontend-new/specs/pad_settings.spec.ts b/src/tests/frontend-new/specs/pad_settings.spec.ts index 9adbd25c185..4f30af259ec 100644 --- a/src/tests/frontend-new/specs/pad_settings.spec.ts +++ b/src/tests/frontend-new/specs/pad_settings.spec.ts @@ -167,6 +167,29 @@ test.describe('creator-owned pad settings', () => { await context2.close(); }); + // #7696: on a short viewport the settings popup must scroll so items in + // Pad-wide Settings (notably "Delete pad") stay reachable instead of being + // cropped off-screen with no scrollbar. + test('settings popup stays scrollable when the viewport is short', async ({page}) => { + await page.setViewportSize({width: 900, height: 500}); + await goToNewPad(page); + await showSettings(page); + + const popupContent = page.locator('#settings > .popup-content'); + await expect(popupContent).toBeVisible(); + await expect(page.locator('#pad-settings-section')).toBeVisible(); + + const dims = await popupContent.evaluate((el) => ({ + overflowY: getComputedStyle(el).overflowY, + scrolls: el.scrollHeight > el.clientHeight, + })); + expect(dims.overflowY).toBe('auto'); + expect(dims.scrolls).toBe(true); + + await page.locator('#delete-pad').scrollIntoViewIfNeeded(); + await expect(page.locator('#delete-pad')).toBeInViewport(); + }); + // #7592: ticking "Disable chat" must visibly disable the dependent // "Chat always on screen" / "Show Chat and Users" toggles, not just // make the underlying inputs non-interactive. From 26cebda3f6ba33beb07bb476c50807360730e711 Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 7 May 2026 20:04:25 +0100 Subject: [PATCH 2/3] fix(7696): float nice-select dropdowns above scrollable popups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo flagged that making .popup-content a scroll container clips absolutely-positioned descendants — so the Settings popup's font and language dropdowns can be truncated when their list extends past the popup's scroll bounds on short viewports. Mirror the existing toolbar workaround: when a nice-select sits inside .popup-content, switch the list to position:fixed (CSS) and place it with viewport-relative coordinates from getBoundingClientRect (JS), respecting the existing reverse class for upward-opening lists. Also relax the regression test per Qodo: drop the brittle scrollHeight > clientHeight assertion in favour of asserting the popup declares overflow-y:auto and proving Delete pad is initially off-screen, then reachable via scroll. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/static/css/pad/form.css | 6 +++++- src/static/js/vendors/nice-select.ts | 16 ++++++++++++++++ .../frontend-new/specs/pad_settings.spec.ts | 19 ++++++++++--------- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/static/css/pad/form.css b/src/static/css/pad/form.css index 5475d7d5c6a..1bd24b8b4f1 100644 --- a/src/static/css/pad/form.css +++ b/src/static/css/pad/form.css @@ -132,7 +132,11 @@ select, .nice-select { bottom: calc(100% + 5px); top: auto; } -.toolbar .nice-select .list { +.toolbar .nice-select .list, +/* Popups are scroll containers (see popup.css), which would otherwise clip the + absolutely-positioned dropdown list. Float it above the popup with fixed + positioning, matching how the toolbar dropdowns escape their container. */ +.popup .nice-select .list { position: fixed; top: auto; left: auto; diff --git a/src/static/js/vendors/nice-select.ts b/src/static/js/vendors/nice-select.ts index ec76b3cef15..1c520f67998 100644 --- a/src/static/js/vendors/nice-select.ts +++ b/src/static/js/vendors/nice-select.ts @@ -123,6 +123,22 @@ } $dropdown.find('.list').css('max-height', $maxListHeight + 'px'); + // Popups are scroll containers (since #7696) which would clip the + // absolutely-positioned dropdown list. The list is repositioned with + // `position: fixed` (see form.css) so it floats above the popup; we + // need viewport-relative coordinates here. Done after the reverse + // class is decided so we know which side of the dropdown to anchor. + if ($dropdown.closest('.toolbar').length === 0 + && $dropdown.closest('.popup-content').length > 0) { + var rect = $dropdown[0].getBoundingClientRect(); + var $list = $dropdown.find('.list'); + $list.css('left', rect.left); + $list.css('min-width', $dropdown.outerWidth() + 'px'); + $list.css('top', $dropdown.hasClass('reverse') + ? rect.top - $maxListHeight - 5 + : rect.bottom); + } + } else { $dropdown.trigger('focus'); } diff --git a/src/tests/frontend-new/specs/pad_settings.spec.ts b/src/tests/frontend-new/specs/pad_settings.spec.ts index 4f30af259ec..b8badd401a6 100644 --- a/src/tests/frontend-new/specs/pad_settings.spec.ts +++ b/src/tests/frontend-new/specs/pad_settings.spec.ts @@ -179,15 +179,16 @@ test.describe('creator-owned pad settings', () => { await expect(popupContent).toBeVisible(); await expect(page.locator('#pad-settings-section')).toBeVisible(); - const dims = await popupContent.evaluate((el) => ({ - overflowY: getComputedStyle(el).overflowY, - scrolls: el.scrollHeight > el.clientHeight, - })); - expect(dims.overflowY).toBe('auto'); - expect(dims.scrolls).toBe(true); - - await page.locator('#delete-pad').scrollIntoViewIfNeeded(); - await expect(page.locator('#delete-pad')).toBeInViewport(); + // The popup must declare scrollable overflow (otherwise the previous bug + // recurs even if content happens to fit by coincidence). + await expect(popupContent).toHaveCSS('overflow-y', 'auto'); + + // Delete pad sits at the bottom of Pad-wide Settings; on a short viewport + // it starts off-screen and must become reachable by scrolling the popup. + const deletePad = page.locator('#delete-pad'); + await expect(deletePad).not.toBeInViewport(); + await deletePad.scrollIntoViewIfNeeded(); + await expect(deletePad).toBeInViewport(); }); // #7592: ticking "Disable chat" must visibly disable the dependent From 3db84877db5e47f36d2b4811db06e088b79d178f Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 7 May 2026 20:17:58 +0100 Subject: [PATCH 3/3] fix(7696): nice-select reverse list disappeared in scrolled popup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a nice-select inside a popup-content scroll container sits in the lower half of the viewport, the JS adds the .reverse class so the list opens upward. The default .reverse rule sets bottom: calc(100% + 5px), which is fine when the list is position:absolute relative to its parent — but with the position:fixed treatment the popup branch uses, that percentage resolves against the viewport and pushes the list ~100vh above the screen, so it appears not to open at all until you scroll to the bottom of the popup (where .reverse no longer triggers). Override the rule for both .toolbar and .popup so .reverse drops back to bottom: auto and JS-set `top` controls placement, with a JS belt-and- braces also setting `bottom: auto` inline. Adds a Playwright regression test that scrolls the settings popup to the bottom, opens the Pad-wide font dropdown, and asserts the list is both visible and inside the viewport. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/static/css/pad/form.css | 8 +++++ src/static/js/vendors/nice-select.ts | 3 ++ .../frontend-new/specs/pad_settings.spec.ts | 29 +++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/src/static/css/pad/form.css b/src/static/css/pad/form.css index 1bd24b8b4f1..8dd9f40da4a 100644 --- a/src/static/css/pad/form.css +++ b/src/static/css/pad/form.css @@ -141,6 +141,14 @@ select, .nice-select { top: auto; left: auto; } +/* The default .reverse rule above sets bottom: calc(100% + 5px) so an + absolutely-positioned list opens upward inside its parent. Once the list is + position:fixed, that percentage resolves against the viewport instead and + would push the list off-screen, so we let JS place it via `top` only. */ +.toolbar .nice-select.reverse .list, +.popup .nice-select.reverse .list { + bottom: auto; +} .nice-select .list:hover .option:not(:hover) { background-color: transparent !important; } diff --git a/src/static/js/vendors/nice-select.ts b/src/static/js/vendors/nice-select.ts index 1c520f67998..4f1ff8ab16d 100644 --- a/src/static/js/vendors/nice-select.ts +++ b/src/static/js/vendors/nice-select.ts @@ -134,6 +134,9 @@ var $list = $dropdown.find('.list'); $list.css('left', rect.left); $list.css('min-width', $dropdown.outerWidth() + 'px'); + // Clear .reverse's `bottom: calc(100% + 5px)` — with position:fixed + // it would resolve against the viewport and push the list offscreen. + $list.css('bottom', 'auto'); $list.css('top', $dropdown.hasClass('reverse') ? rect.top - $maxListHeight - 5 : rect.bottom); diff --git a/src/tests/frontend-new/specs/pad_settings.spec.ts b/src/tests/frontend-new/specs/pad_settings.spec.ts index b8badd401a6..1fbd74f86d8 100644 --- a/src/tests/frontend-new/specs/pad_settings.spec.ts +++ b/src/tests/frontend-new/specs/pad_settings.spec.ts @@ -191,6 +191,35 @@ test.describe('creator-owned pad settings', () => { await expect(deletePad).toBeInViewport(); }); + // #7696 follow-up: the Pad-wide font/language nice-select dropdowns sit + // near the bottom of the popup, so opening one triggers the .reverse path + // (open upward). Floating the list with position:fixed must not pick up + // the default `.reverse { bottom: calc(100% + 5px) }` rule, which would + // resolve against the viewport and place the list off-screen. + test('Pad-wide font dropdown opens visibly when popup is scrolled to bottom', async ({page}) => { + await page.setViewportSize({width: 900, height: 500}); + await goToNewPad(page); + await showSettings(page); + + // Force the font dropdown into the lower portion of the viewport so + // .reverse triggers and the list opens upward. + await page.locator('#settings > .popup-content').evaluate((el) => { + el.scrollTop = el.scrollHeight; + }); + + const fontDropdown = page.locator('#padsettings-viewfontmenu + .nice-select'); + await expect(fontDropdown).toBeInViewport(); + + await fontDropdown.click(); + const list = fontDropdown.locator('.list'); + await expect(list).toBeVisible(); + await expect(list).toBeInViewport(); + + // The first option must be reachable so users can actually pick a font. + await fontDropdown.locator('.option').first().click(); + await expect(fontDropdown).not.toHaveClass(/open/); + }); + // #7592: ticking "Disable chat" must visibly disable the dependent // "Chat always on screen" / "Show Chat and Users" toggles, not just // make the underlying inputs non-interactive.