feat(web): implement subkey menu realignment for even columns (#9768)

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; i<nKeys; i++, iCol++) {
+    let iCol = 0;
+    let rowColumnCounts: number[] = []; // Track columns per row
+    for(let i=0; i<nKeys; i++) {
       let subkeyWidth = (typeof subKeySpec[i]['width'] != 'undefined') ?
         subKeySpec[i]['width'] * e.offsetWidth / 100 :
         e.offsetWidth;
@@ -173,6 +176,7 @@ export default class SubkeyPopup implements GestureHandler {
         // TODO: currently we don't check that the rows fit vertically,
         // so it's possible that the top or bottom of the subkey menu
         // is not visible.
+        rowColumnCounts[iRow] = iCol; // Save column count for the completed row
         iRow++;
         iCol = 0;
         thisRowWidth = SUBKEY_DEFAULT_MARGIN_LEFT;
@@ -184,7 +188,12 @@ export default class SubkeyPopup implements GestureHandler {
       this.subkeys.push(kDiv.firstChild as KeyElement);
 
       elements.appendChild(kDiv);
+      iCol++;
     }
+    // Save column count for the last row
+    rowColumnCounts[iRow] = iCol;
+    // Find the maximum row column count (the widest row determines menu width and shift logic)
+    this.maxRowColumns = Math.max(...rowColumnCounts);
 
     ss.width = this.menuWidth + 'px';
 
@@ -307,6 +316,30 @@ export default class SubkeyPopup implements GestureHandler {
     const ss=subKeys.style;
     const parentOffsetLeft = e.offsetParent ? (<HTMLElement>e.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) {

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('<!DOCTYPE html><p>Hello world</p>');
+    global.document = dom.window.document;
+    global.getComputedStyle = dom.window.getComputedStyle;
+  });
+
+  const DEFAULT_KEY = -1;
+
+  const mockSource = () => {
+    const gestureSourceSubview = new GestureSourceSubview<KeyElement>(
+      { path: new GesturePath<KeyElement, any>() } as GestureSource<KeyElement>,
+      {} as typeof GestureSource.prototype['recognizerConfigStack'], false, null);
+    return {
+      stageReports: [{ sources: [gestureSourceSubview] }],
+      on: (event, callback) => { },
+    } as GestureSequence<KeyElement, string>;
+  }
+
+  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');
+  });
+
+});