diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 3275d9f4..452d5705 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -341,6 +341,7 @@ jobs: CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-add-modal-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-share-color-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-ws-batch-e2e.js 2>&1 | tee -a e2e-output.txt + CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1487-byop-modal-layout-e2e.js 2>&1 | tee -a e2e-output.txt - name: Collect frontend coverage (parallel) if: success() && github.event_name == 'push' diff --git a/public/style.css b/public/style.css index 6b873ef5..5dfdd9a9 100644 --- a/public/style.css +++ b/public/style.css @@ -1333,18 +1333,45 @@ body.scroll-locked { overflow: hidden; } .byop-modal .byop-header { position: sticky; top: 0; z-index: 2; background: var(--card-bg); - margin: -24px -24px 12px; - padding: 12px 24px; + /* #1487: was `margin: -24px -24px 12px; padding: 12px 24px;` — assumed + .modal desktop padding of 24px. On mobile .modal switches to 16px + padding, so the negative margin overshot, content-flow pushed the + next sibling UP under the (sticky-pinned) header, and the + description paragraph was visually occluded by ~12px. Drop the + full-bleed negative-margin gymnastics — header now uses normal + padding (modal padding handles the inset) and bounded sizing. */ + margin: 0 0 12px; + padding: 4px 0; border-bottom: 1px solid var(--border); display: flex; align-items: center; justify-content: space-between; gap: 8px; + /* Hard cap so emoji ascent / btn-icon 48px floor can't blow the header + past the safe range (was 73px pre-fix). */ + max-height: 48px; + box-sizing: border-box; +} +.byop-modal .byop-header h3 { + /* #1487: explicit font + line-height so emoji-bearing title doesn't + blow up the header on platforms where the default emoji font has + huge ascent metrics (observed contributor to 73px header). */ + margin: 0; font-size: 1rem; line-height: 1.3; } /* MINOR-7: dropped position: sticky on .byop-x — it sits inside the already-sticky .byop-header, so the sticky on the child was a no-op. - We only need visual styling here. */ + We only need visual styling here. + #1487: override .btn-icon 48px floor — inside the header we need a + compact button so the header stays bounded. The :before tap-target + pad below preserves accessibility (effective hit area ~ 44x44). */ .byop-modal .byop-x { background: var(--card-bg); border: 1px solid var(--border); - border-radius: 6px; padding: 4px 10px; cursor: pointer; font-size: 16px; + border-radius: 6px; padding: 2px 8px; cursor: pointer; font-size: 14px; line-height: 1; color: var(--text); + min-height: 0; min-width: 0; height: 32px; + position: relative; +} +/* #1487: invisible tap-pad to keep ≥44px effective hit area while the + visual button stays compact inside the bounded header. */ +.byop-modal .byop-x::before { + content: ''; position: absolute; inset: -8px; border-radius: 8px; } .byop-modal .byop-x:hover { background: var(--row-hover, rgba(0,0,0,0.05)); } diff --git a/test-issue-1487-byop-modal-layout-e2e.js b/test-issue-1487-byop-modal-layout-e2e.js new file mode 100644 index 00000000..ddad3aa9 --- /dev/null +++ b/test-issue-1487-byop-modal-layout-e2e.js @@ -0,0 +1,127 @@ +/** + * E2E (#1487): BYOP modal must render with a usable layout — the title bar + * must NOT consume most of the dialog height and the body controls + * (description, textarea, Decode button) must be visible and not occluded + * by the sticky header. + * + * Reporter: @EldoonNemar — "The dialog text can't be seen due to the title + * bar being massive." + * + * Repro: + * 1. Open /#/packets on mobile (390x844). + * 2. Click the 📦 BYOP toolbar button. + * 3. Observe the modal: the .byop-header swells (~73px tall) and the + * next-sibling description paragraph (`.text-muted`) starts INSIDE + * the sticky-header band, getting visually clipped/occluded. + * + * Root cause: `.byop-header` uses `position: sticky` + a negative + * `margin: -24px -24px 12px` that assumes desktop `.modal` padding of + * 24px — but `.modal` switches to 16px padding on mobile. The close + * button's box (border + padding) further inflates the header. The + * description paragraph then begins at top≈85 inside a header that + * spans 24–97, hiding the text. + * + * Fix expectation: + * - Header height is bounded (<= 56px is a reasonable target). + * - The description paragraph's top edge is BELOW the sticky-header + * bottom edge — i.e. no visual occlusion. + * - The textarea and Decode button are fully within the modal client rect. + * + * Usage: BASE_URL=http://localhost:13581 node test-issue-1487-byop-modal-layout-e2e.js + */ +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' ✓ ' + name); } + catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); } +} +function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +async function probeModal(page) { + return page.evaluate(() => { + const m = document.querySelector('.byop-modal'); + if (!m) return { err: 'no modal' }; + const hdr = m.querySelector('.byop-header'); + const desc = m.querySelector('.text-muted'); + const ta = m.querySelector('.byop-input'); + const btn = m.querySelector('#byopDecode'); + const mr = m.getBoundingClientRect(); + const hr = hdr.getBoundingClientRect(); + const dr = desc.getBoundingClientRect(); + const tr = ta.getBoundingClientRect(); + const br = btn.getBoundingClientRect(); + return { + modalH: Math.round(mr.height), + hdrH: Math.round(hr.height), + hdrBottom: Math.round(hr.bottom), + descTop: Math.round(dr.top), + taBottom: Math.round(tr.bottom), + btnBottom: Math.round(br.bottom), + modalBottom: Math.round(mr.bottom), + }; + }); +} + +async function openBYOP(page) { + await page.waitForSelector('[data-action="pkt-byop"]', { timeout: 8000 }); + await page.evaluate(() => { + document.querySelectorAll('.byop-overlay').forEach(o => o.remove()); + document.querySelector('[data-action="pkt-byop"]').click(); + }); + await page.waitForSelector('.byop-modal', { timeout: 5000 }); + await page.waitForTimeout(200); +} + +async function runViewport(browser, label, viewport) { + const ctx = await browser.newContext({ viewport }); + const page = await ctx.newPage(); + page.setDefaultTimeout(10000); + page.on('pageerror', (e) => console.error('[pageerror]', e.message)); + + console.log(`\n--- viewport ${label} (${viewport.width}x${viewport.height}) ---`); + + await step('navigate to /packets and open BYOP', async () => { + await page.goto(BASE + '/#/packets', { waitUntil: 'domcontentloaded' }); + await openBYOP(page); + }); + + await step('header height is bounded (<= 56px)', async () => { + const p = await probeModal(page); + assert(p.hdrH <= 56, `header height ${p.hdrH}px > 56px cap (modal=${p.modalH}px)`); + }); + + await step('description paragraph is NOT occluded by sticky header', async () => { + const p = await probeModal(page); + assert(p.descTop >= p.hdrBottom, + `description top (${p.descTop}) starts INSIDE sticky header band (header bottom=${p.hdrBottom})`); + }); + + await step('textarea and Decode button do not overflow modal client rect', async () => { + const p = await probeModal(page); + assert(p.taBottom <= p.modalBottom + 1, `textarea bottom (${p.taBottom}) overflows modal (${p.modalBottom})`); + assert(p.btnBottom <= p.modalBottom + 1, `Decode button bottom (${p.btnBottom}) overflows modal`); + }); + + await ctx.close(); +} + +(async () => { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + + console.log(`\n=== #1487 BYOP modal layout E2E against ${BASE} ===`); + + await runViewport(browser, 'mobile', { width: 390, height: 844 }); + await runViewport(browser, 'desktop', { width: 1280, height: 800 }); + + await browser.close(); + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed === 0 ? 0 : 1); +})().catch(e => { console.error(e); process.exit(1); });