From 637f5c45ce489c92be1dc686a2b60cf7ecf1038d Mon Sep 17 00:00:00 2001 From: azmy60 Date: Tue, 12 May 2026 09:21:17 +0700 Subject: [PATCH 1/3] fix overflowing menu (cherry picked from commit c4ac9c55fcbe999e0092fe2e21dada68e51886e8) --- playwright.config.js | 4 +-- src/js/core/tools/Popup.js | 11 +++++-- test/e2e/basic.spec.js | 4 +-- test/e2e/menu.html | 61 ++++++++++++++++++++++++++++++++++++++ test/e2e/menu.spec.js | 47 +++++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 8 deletions(-) create mode 100644 test/e2e/menu.html create mode 100644 test/e2e/menu.spec.js diff --git a/playwright.config.js b/playwright.config.js index 9aec529a7..68a450b18 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -43,8 +43,8 @@ export default defineConfig({ /* Run your local dev server before starting the tests */ webServer: { - command: 'npx serve test -p 3000', - port: 3000, + command: 'npx serve . -p 9876', + port: 9876, timeout: 60 * 1000, reuseExistingServer: !process.env.CI, }, diff --git a/src/js/core/tools/Popup.js b/src/js/core/tools/Popup.js index d2ec9b3f4..bde739b97 100644 --- a/src/js/core/tools/Popup.js +++ b/src/js/core/tools/Popup.js @@ -203,13 +203,18 @@ export default class Popup extends CoreFeature{ case "bottom": this.element.style.top = (parseInt(this.element.style.top) - this.element.offsetHeight - parentEl.offsetHeight - 1) + "px"; break; - + default: this.element.style.top = (parseInt(this.element.style.top) - this.element.offsetHeight + parentEl.offsetHeight + 1) + "px"; } - + }else{ - this.element.style.height = offsetHeight + "px"; + let newTop = y - this.element.offsetHeight; + if(newTop < 0){ + newTop = 0; + this.element.style.height = offsetHeight + "px"; + } + this.element.style.top = newTop + "px"; } } } diff --git a/test/e2e/basic.spec.js b/test/e2e/basic.spec.js index 3a24096a4..69a1b4a0b 100644 --- a/test/e2e/basic.spec.js +++ b/test/e2e/basic.spec.js @@ -1,11 +1,9 @@ // @ts-check import { test, expect } from "@playwright/test"; -import { join } from "path"; test.describe("Tabulator functionality", () => { test.beforeEach(async ({ page }) => { - const htmlPath = join(__dirname, "index.html"); - await page.goto(`file://${htmlPath}`); + await page.goto("/test/e2e/index.html"); await page.waitForSelector(".tabulator"); }); diff --git a/test/e2e/menu.html b/test/e2e/menu.html new file mode 100644 index 000000000..4e9bf261e --- /dev/null +++ b/test/e2e/menu.html @@ -0,0 +1,61 @@ + + + + + Tabulator Menu Test + + + + + +
+ + + + diff --git a/test/e2e/menu.spec.js b/test/e2e/menu.spec.js new file mode 100644 index 000000000..293512a45 --- /dev/null +++ b/test/e2e/menu.spec.js @@ -0,0 +1,47 @@ +// @ts-check +import { test, expect } from "@playwright/test"; + +test.describe("Context menu viewport bounds", () => { + test.beforeEach(async ({ page }) => { + await page.goto("/test/e2e/menu.html"); + await page.waitForSelector(".tabulator-row"); + }); + + const margin = 5; + const corners = [ + { name: "top-left", offsetX: margin, offsetY: 50 }, + { name: "top-right", offsetX: -margin, offsetY: 50 }, + { name: "bottom-left", offsetX: margin, offsetY: -margin }, + { name: "bottom-right", offsetX: -margin, offsetY: -margin }, + ]; + + for (const corner of corners) { + test(`menu stays inside viewport when opened near ${corner.name}`, async ({ page }) => { + const viewport = page.viewportSize(); + const x = corner.offsetX < 0 ? viewport.width + corner.offsetX : corner.offsetX; + const y = corner.offsetY < 0 ? viewport.height + corner.offsetY : corner.offsetY; + + await page.evaluate(({ x, y }) => { + const el = document.elementFromPoint(x, y); + el.dispatchEvent(new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + view: window, + button: 2, + clientX: x, + clientY: y, + })); + }, { x, y }); + + const menu = page.locator(".tabulator-menu"); + await expect(menu).toBeVisible(); + + const box = await menu.boundingBox(); + expect(box).not.toBeNull(); + expect(box.x).toBeGreaterThanOrEqual(0); + expect(box.y).toBeGreaterThanOrEqual(0); + expect(box.x + box.width).toBeLessThanOrEqual(viewport.width); + expect(box.y + box.height).toBeLessThanOrEqual(viewport.height); + }); + } +}); From 2e3228e32ae657ecebb9bcc50a90ecb416e17e75 Mon Sep 17 00:00:00 2001 From: azmy60 Date: Tue, 12 May 2026 09:37:43 +0700 Subject: [PATCH 2/3] add e2e test for menu viewport overflow and refine fit logic Popup._fitToScreen now flips a popup upward when it would overflow the container bottom, anchoring the menu's bottom to the container when a flip-up would push it past the top edge. Only falls back to the full-height + scroll behavior when the menu itself is taller than the container, which avoids stretching the menu across the viewport for mid-screen triggers. Co-Authored-By: Claude Opus 4.7 (1M context) (cherry picked from commit c7e1321a5d6995d3f0deffeb0b99deae0b977675) --- playwright.config.js | 2 +- src/js/core/tools/Popup.js | 13 +++++++++---- test/e2e/menu.spec.js | 13 +++++++++---- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/playwright.config.js b/playwright.config.js index 68a450b18..606c74daa 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -27,7 +27,7 @@ export default defineConfig({ /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Base URL to use in actions like `await page.goto('/')`. */ - // baseURL: 'http://127.0.0.1:3000', + baseURL: 'http://127.0.0.1:9876', /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ trace: 'on-first-retry', diff --git a/src/js/core/tools/Popup.js b/src/js/core/tools/Popup.js index bde739b97..796a51b7b 100644 --- a/src/js/core/tools/Popup.js +++ b/src/js/core/tools/Popup.js @@ -209,12 +209,17 @@ export default class Popup extends CoreFeature{ } }else{ - let newTop = y - this.element.offsetHeight; - if(newTop < 0){ - newTop = 0; + let menuHeight = this.element.offsetHeight; + if(menuHeight > offsetHeight){ + this.element.style.top = "0px"; this.element.style.height = offsetHeight + "px"; + }else{ + let newTop = y - menuHeight; + if(newTop < 0){ + newTop = offsetHeight - menuHeight; + } + this.element.style.top = newTop + "px"; } - this.element.style.top = newTop + "px"; } } } diff --git a/test/e2e/menu.spec.js b/test/e2e/menu.spec.js index 293512a45..7fe897846 100644 --- a/test/e2e/menu.spec.js +++ b/test/e2e/menu.spec.js @@ -8,18 +8,23 @@ test.describe("Context menu viewport bounds", () => { }); const margin = 5; - const corners = [ + const positions = [ { name: "top-left", offsetX: margin, offsetY: 50 }, { name: "top-right", offsetX: -margin, offsetY: 50 }, { name: "bottom-left", offsetX: margin, offsetY: -margin }, { name: "bottom-right", offsetX: -margin, offsetY: -margin }, + { name: "middle", offsetX: 0.5, offsetY: 0.5 }, ]; - for (const corner of corners) { + for (const corner of positions) { test(`menu stays inside viewport when opened near ${corner.name}`, async ({ page }) => { const viewport = page.viewportSize(); - const x = corner.offsetX < 0 ? viewport.width + corner.offsetX : corner.offsetX; - const y = corner.offsetY < 0 ? viewport.height + corner.offsetY : corner.offsetY; + const resolve = (offset, size) => { + if (offset > 0 && offset < 1) return Math.round(size * offset); + return offset < 0 ? size + offset : offset; + }; + const x = resolve(corner.offsetX, viewport.width); + const y = resolve(corner.offsetY, viewport.height); await page.evaluate(({ x, y }) => { const el = document.elementFromPoint(x, y); From d041486dfdf55a1574504559421626eb85d3d1e0 Mon Sep 17 00:00:00 2001 From: azmy60 Date: Fri, 5 Jun 2026 11:06:25 +0700 Subject: [PATCH 3/3] restore unnecessary changes --- playwright.config.js | 6 +++--- test/e2e/basic.spec.js | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/playwright.config.js b/playwright.config.js index 606c74daa..9aec529a7 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -27,7 +27,7 @@ export default defineConfig({ /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Base URL to use in actions like `await page.goto('/')`. */ - baseURL: 'http://127.0.0.1:9876', + // baseURL: 'http://127.0.0.1:3000', /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ trace: 'on-first-retry', @@ -43,8 +43,8 @@ export default defineConfig({ /* Run your local dev server before starting the tests */ webServer: { - command: 'npx serve . -p 9876', - port: 9876, + command: 'npx serve test -p 3000', + port: 3000, timeout: 60 * 1000, reuseExistingServer: !process.env.CI, }, diff --git a/test/e2e/basic.spec.js b/test/e2e/basic.spec.js index 69a1b4a0b..3a24096a4 100644 --- a/test/e2e/basic.spec.js +++ b/test/e2e/basic.spec.js @@ -1,9 +1,11 @@ // @ts-check import { test, expect } from "@playwright/test"; +import { join } from "path"; test.describe("Tabulator functionality", () => { test.beforeEach(async ({ page }) => { - await page.goto("/test/e2e/index.html"); + const htmlPath = join(__dirname, "index.html"); + await page.goto(`file://${htmlPath}`); await page.waitForSelector(".tabulator"); });