diff --git a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts index dcfd45ca9b8..a0e4d39b29b 100644 --- a/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts +++ b/web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts @@ -61,6 +61,7 @@ export default class SubkeyPopup implements GestureHandler { private callout: HTMLDivElement; private readonly menuWidth: number; + private readonly maxRowColumns: number; public readonly baseKey: KeyElement; public readonly subkeys: KeyElement[]; @@ -159,7 +160,9 @@ export default class SubkeyPopup implements GestureHandler { this.subkeys = []; let thisRowWidth = SUBKEY_DEFAULT_MARGIN_LEFT; let iRow = 0; - for(let i=0, iCol=0; ie.offsetParent).offsetLeft : 0; let x = e.offsetLeft + parentOffsetLeft + 0.5*(e.offsetWidth-subKeys.offsetWidth); + + // Issue #9768: Realign subkey menu when columns are even to avoid ambiguous default selection + // With even-numbered columns, shift the menu by half a key width to ensure one option + // sits unambiguously under the default touch position (like Google's Gboard) + // Note: We check the widest row (which determines the menu width), not the idealized column + // count. Since rows are left-aligned within the popup, the widest row determines the overall + // centering. For uneven distributions (e.g., 11 keys as 6+5), the widest row has 6 columns + // (even), so we apply the shift even though the bottom row has 5 (odd), because all rows are + // left-aligned and the shorter bottom row is also offset left from the base key center. + // In rare constrained cases (3+ row popup on top keyboard row where the popup shifts down), + // a middle row may actually be closest to the finger. However, since the entire popup shifts + // horizontally as a unit, there will still be a key positioned under the finger. + // This assumes uniform key widths. If subkeys have significantly different widths, + // the actual layout may already be unambiguous, but applying this shift won't cause harm + // (bounds checking prevents overflow). + if (this.maxRowColumns % 2 === 0) { + const keyCenter = e.offsetLeft + parentOffsetLeft + e.offsetWidth / 2; + const keyboardCenter = vkbd.width / 2; + const halfKeyShift = e.offsetWidth / 2; + + // Shift left if key is on right side, right if on left side + x += (keyCenter > keyboardCenter) ? -halfKeyShift : halfKeyShift; + } + const xMax = vkbd.width - subKeys.offsetWidth; if(x > xMax) { diff --git a/web/src/test/auto/headless/engine/osk/input/gestures/browser/subkeyPopup.tests.ts b/web/src/test/auto/headless/engine/osk/input/gestures/browser/subkeyPopup.tests.ts index 72679897f37..621dd117b6b 100644 --- a/web/src/test/auto/headless/engine/osk/input/gestures/browser/subkeyPopup.tests.ts +++ b/web/src/test/auto/headless/engine/osk/input/gestures/browser/subkeyPopup.tests.ts @@ -198,3 +198,272 @@ describe('subkey menu width', () => { }); }); + +// Tests for #9768: even-column realignment +describe('subkey menu even-column realignment', () => { + let dom: JSDOM; + + beforeEach(() => { + dom = new JSDOM('

Hello world

'); + global.document = dom.window.document; + global.getComputedStyle = dom.window.getComputedStyle; + }); + + const DEFAULT_KEY = -1; + + const mockSource = () => { + const gestureSourceSubview = new GestureSourceSubview( + { path: new GesturePath() } as GestureSource, + {} as typeof GestureSource.prototype['recognizerConfigStack'], false, null); + return { + stageReports: [{ sources: [gestureSourceSubview] }], + on: (event, callback) => { }, + } as GestureSequence; + } + + const mockVisualKeyboard = (oskWidth: number) => { + const topContainer = document.createElement('div'); + document.body.appendChild(topContainer); + + const oskElement = document.createElement('div'); + topContainer.appendChild(oskElement); + + const visualKeyboard = sinon.createStubInstance(VisualKeyboard); + sinon.stub(visualKeyboard, 'device').get(() => new DeviceSpec('Chrome', 'Phone', 'Android', false)); + sinon.stub(visualKeyboard, 'topContainer').get(() => topContainer); + sinon.stub(visualKeyboard, 'element').get(() => oskElement); + sinon.stub(visualKeyboard, 'isEmbedded').get(() => false); + sinon.stub(visualKeyboard, 'layerId').get(() => '_default'); + sinon.stub(visualKeyboard, 'width').get(() => oskWidth); + + (visualKeyboard as any).kbdDiv = null; + sinon.stub(visualKeyboard, 'kbdDiv').value(document.createElement('div')); + return visualKeyboard; + } + + const mockKeys = (offsetLeft: number, width: number, height: number, subkeySpecs: number[]) => { + const rowElement = document.createElement('div'); + document.body.appendChild(rowElement); + + const row = sinon.createStubInstance(OSKRow); + (row as any).element = null; + sinon.stub(row, 'element').value(rowElement); + + const baseKey = sinon.createStubInstance(OSKBaseKey); + (baseKey as any).row = null; + sinon.stub(baseKey, 'row').value(row); + + const subKeys = []; + for (const subkeyWidth of subkeySpecs) { + const subKey = sinon.createStubInstance(ActiveSubKey); + if (subkeyWidth >= 0) { + (subKey as any).width = null; + sinon.stub(subKey, 'width').get(() => subkeyWidth); + } + subKeys.push(subKey); + } + + const key = link(document.createElement('div'), { key: baseKey, keyId: 'test-key', subKeys: subKeys }); + sinon.stub(key, 'offsetLeft').get(() => offsetLeft); + sinon.stub(key, 'offsetWidth').get(() => width); + sinon.stub(key, 'offsetHeight').get(() => height); + sinon.stub(key, 'offsetParent').get(() => null); + + return key; + } + + it('Even columns (2) on left side shifts menu right', () => { + // Key on left side of keyboard (offsetLeft=10, keyboard width=300) + const sut = new SubkeyPopup( + mockSource(), + sinon.stub(), + mockVisualKeyboard(300 /*px*/), + mockKeys(10, 50, 40, [DEFAULT_KEY, DEFAULT_KEY]), + DEFAULT_GESTURE_PARAMS + ); + + // Insert element into DOM to get computed width + document.body.appendChild(sut.element); + + // Manually set the menu width to simulate layout + sut.element.style.width = '110px'; // 2 * (50 + 5) + 5 + Object.defineProperty(sut.element, 'offsetWidth', { value: 110, configurable: true }); + + // Reposition to apply the alignment logic + sut.reposition(mockVisualKeyboard(300)); + + const menuLeft = parseInt(sut.element.style.left); + + // Base position would be: 10 + 0.5*(50-110) = 10 - 30 = -20 + // With even-column shift right: -20 + 25 = 5 + // But clamped to >= 0, so expected = 5 + assert.isAtLeast(menuLeft, 0, 'Menu should not overflow left edge'); + assert.isAtMost(menuLeft, 190, 'Menu should not overflow right edge (300 - 110)'); + + // The key is on the left side, so we expect a rightward shift + // Without shift: centered would be at (10 + 25 - 55) = -20, clamped to 0 + // With shift: (-20 + 25) = 5, clamped to 5 + assert.equal(menuLeft, 5); + }); + + it('Even columns (2) on right side shifts menu left', () => { + // Key on right side of keyboard (offsetLeft=240, keyboard width=300) + const sut = new SubkeyPopup( + mockSource(), + sinon.stub(), + mockVisualKeyboard(300 /*px*/), + mockKeys(240, 50, 40, [DEFAULT_KEY, DEFAULT_KEY]), + DEFAULT_GESTURE_PARAMS + ); + + document.body.appendChild(sut.element); + sut.element.style.width = '110px'; + Object.defineProperty(sut.element, 'offsetWidth', { value: 110, configurable: true }); + + sut.reposition(mockVisualKeyboard(300)); + + const menuLeft = parseInt(sut.element.style.left); + + // Base position: 240 + 0.5*(50-110) = 240 - 30 = 210 + // With even-column shift left: 210 - 25 = 185 + // Max allowed: 300 - 110 = 190, so 185 is within bounds + assert.equal(menuLeft, 185); + }); + + it('Odd columns (3) remains centered regardless of position', () => { + // Key on left side + const sutLeft = new SubkeyPopup( + mockSource(), + sinon.stub(), + mockVisualKeyboard(300 /*px*/), + mockKeys(10, 50, 40, [DEFAULT_KEY, DEFAULT_KEY, DEFAULT_KEY]), + DEFAULT_GESTURE_PARAMS + ); + + document.body.appendChild(sutLeft.element); + sutLeft.element.style.width = '165px'; // 3 * (50 + 5) + 5 + Object.defineProperty(sutLeft.element, 'offsetWidth', { value: 165, configurable: true }); + + sutLeft.reposition(mockVisualKeyboard(300)); + + const menuLeftPos = parseInt(sutLeft.element.style.left); + + // Base position: 10 + 0.5*(50-165) = 10 - 57.5 = -47.5 + // No shift for odd columns, clamped to 0 + assert.equal(menuLeftPos, 0); + + // Key on right side + const sutRight = new SubkeyPopup( + mockSource(), + sinon.stub(), + mockVisualKeyboard(300 /*px*/), + mockKeys(240, 50, 40, [DEFAULT_KEY, DEFAULT_KEY, DEFAULT_KEY]), + DEFAULT_GESTURE_PARAMS + ); + + document.body.appendChild(sutRight.element); + sutRight.element.style.width = '165px'; + Object.defineProperty(sutRight.element, 'offsetWidth', { value: 165, configurable: true }); + + sutRight.reposition(mockVisualKeyboard(300)); + + const menuRightPos = parseInt(sutRight.element.style.left); + + // Base position: 240 + 0.5*(50-165) = 240 - 57.5 = 182.5 + // Max allowed: 300 - 165 = 135, so clamped to 135 + assert.equal(menuRightPos, 135); + }); + + it('Even columns (4) applies shift correctly', () => { + // 4 subkeys = 2 columns (max 9 per row, ceil(4/1) = 4, but width constraint may wrap) + // For this test, assume they fit in 2x2 layout + const sut = new SubkeyPopup( + mockSource(), + sinon.stub(), + mockVisualKeyboard(400 /*px*/), + mockKeys(100, 50, 40, [DEFAULT_KEY, DEFAULT_KEY, DEFAULT_KEY, DEFAULT_KEY]), + DEFAULT_GESTURE_PARAMS + ); + + document.body.appendChild(sut.element); + + // With 4 keys: nRows = ceil(4/9) = 1, nCols = ceil(4/1) = 4 + // So this will have 4 columns (even), not 2 + // Width: 4 * (50 + 5) + 5 = 225 + sut.element.style.width = '225px'; + Object.defineProperty(sut.element, 'offsetWidth', { value: 225, configurable: true }); + + sut.reposition(mockVisualKeyboard(400)); + + const menuLeft = parseInt(sut.element.style.left); + + // Key center: 100 + 25 = 125 + // Keyboard center: 200 + // Key is on left side (125 < 200), so shift right + // Base position: 100 + 0.5*(50-225) = 100 - 87.5 = 12.5 + // With shift: 12.5 + 25 = 37.5 -> 37 (rounded) + assert.isAtLeast(menuLeft, 0); + assert.isAtMost(menuLeft, 175); // 400 - 225 + }); + + it('Uneven rows (11 keys: 6+5) applies shift when widest row is even', () => { + // 11 keys: nRows = ceil(11/9) = 2, nCols = ceil(11/2) = 6 + // Layout: Top row has 6 keys (even), bottom row has 5 keys (odd) + // Rows are left-aligned, so widest row (6, even) determines shift + const sut = new SubkeyPopup( + mockSource(), + sinon.stub(), + mockVisualKeyboard(400 /*px*/), + mockKeys(100, 50, 40, Array(11).fill(DEFAULT_KEY)), + DEFAULT_GESTURE_PARAMS + ); + + document.body.appendChild(sut.element); + + // Width for 6 keys: 6 * (50 + 5) + 5 = 335 + sut.element.style.width = '335px'; + Object.defineProperty(sut.element, 'offsetWidth', { value: 335, configurable: true }); + + sut.reposition(mockVisualKeyboard(400)); + + const menuLeft = parseInt(sut.element.style.left); + + // Key center: 100 + 25 = 125 + // Keyboard center: 200 + // Key is on left side, widest row has even columns (6), so shift RIGHT + // Base position: 100 + 0.5*(50-335) = 100 - 142.5 = -42.5 + // With shift: -42.5 + 25 = -17.5, clamped to 0 + assert.equal(menuLeft, 0, 'Should apply rightward shift but be clamped to 0'); + }); + + it('Uneven rows (12 keys: 6+6) applies shift when widest row is even', () => { + // 12 keys: nRows = ceil(12/9) = 2, nCols = ceil(12/2) = 6 + // Layout: Top row has 6 keys (even), bottom row has 6 keys (even) + // Widest row is 6 (even), shift SHOULD be applied + const sut = new SubkeyPopup( + mockSource(), + sinon.stub(), + mockVisualKeyboard(400 /*px*/), + mockKeys(100, 50, 40, Array(12).fill(DEFAULT_KEY)), + DEFAULT_GESTURE_PARAMS + ); + + document.body.appendChild(sut.element); + + // Width for 6 keys: 6 * (50 + 5) + 5 = 335 + sut.element.style.width = '335px'; + Object.defineProperty(sut.element, 'offsetWidth', { value: 335, configurable: true }); + + sut.reposition(mockVisualKeyboard(400)); + + const menuLeft = parseInt(sut.element.style.left); + + // Key center: 100 + 25 = 125 + // Keyboard center: 200 + // Key is on left side, widest row has even columns, so shift right + // Base position: 100 + 0.5*(50-335) = 100 - 142.5 = -42.5 + // With shift: -42.5 + 25 = -17.5, clamped to 0 + assert.equal(menuLeft, 0, 'Should apply rightward shift but be clamped to 0'); + }); + +});