Skip to content

Commit

Permalink
fix(web): Small refactorings based on code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@ermshiperete
ermshiperete committed Apr 24, 2024
1 parent 0e14939 commit eb9b34c
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 27 deletions.
28 changes: 11 additions & 17 deletions web/src/engine/osk/src/input/gestures/browser/subkeyPopup.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,28 +142,22 @@ export default class SubkeyPopup implements GestureHandler {
ss.fontSize=computedStyle.fontSize;
ss.visibility='hidden';

let layer = e['key'].layer;
if (typeof (layer) != 'string' || layer == '') {
// Use the currently-active layer.
layer = vkbd.layerId;
}

const nKeys = subKeySpec.length;
// If we have more than 9 keys we put them in (at least) two rows.
const nRows=Math.min(Math.ceil(nKeys/9),2);
// Put a maximum of 9 keys in a row to reduce travel distance
const nRows=Math.ceil(nKeys/9);
const nCols=Math.ceil(nKeys/nRows);

// Add nested button elements for each sub-key
this.subkeys = [];
let thisRowWidth = 0;
let thisRowWidth = SUBKEY_DEFAULT_MARGIN_LEFT;
let iRow = 0;
for(let i=0, iCol=0; i<nKeys; i++, iCol++) {
if (iCol >= nCols) {
iRow++;
iCol = 0;
thisRowWidth = 0;
}

let layer = e['key'].layer;
if(typeof(layer) != 'string' || layer == '') {
// Use the currently-active layer.
layer = vkbd.layerId;
}

let subkeyWidth = (typeof subKeySpec[i]['width'] != 'undefined') ?
subKeySpec[i]['width'] * e.offsetWidth / 100 :
e.offsetWidth;
Expand All @@ -172,14 +166,14 @@ export default class SubkeyPopup implements GestureHandler {
subkeyWidth = vkbd.width - 2 * SUBKEY_DEFAULT_MARGIN_LEFT;
}

if (thisRowWidth + subkeyWidth + SUBKEY_DEFAULT_MARGIN_LEFT > vkbd.width) {
if (thisRowWidth + subkeyWidth + SUBKEY_DEFAULT_MARGIN_LEFT > vkbd.width || iCol >= nCols) {
// New subkey doesn't fit in the current row. Start a new row.
// 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.
iRow++;
iCol = 0;
thisRowWidth = 0;
thisRowWidth = SUBKEY_DEFAULT_MARGIN_LEFT;
}
const keyGenerator = new OSKSubKey(subKeySpec[i], layer);
const kDiv = keyGenerator.construct(vkbd, <KeyElement>e, subkeyWidth, iRow > 0);
Expand Down
53 changes: 43 additions & 10 deletions web/src/test/auto/headless/engine/osk/input/gestures/browser/subkeyPopup.tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('subkey menu width', () => {
DEFAULT_GESTURE_PARAMS
);

assert.equal(sut.element.style.width, '30px' /* 25 + SUBKEY_DEFAULT_MARGIN_LEFT */);
assert.equal(sut.element.style.width, '35px' /* 25 + 2 * SUBKEY_DEFAULT_MARGIN_LEFT */);
assert.equal((sut.element.children[0] as HTMLDivElement).style.marginTop, '');
});

Expand All @@ -92,7 +92,7 @@ describe('subkey menu width', () => {
);

// should be 2 rows with 5 keys each
assert.equal(sut.element.style.width, '150px' /* 5 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) */);
assert.equal(sut.element.style.width, '155px' /* 5 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + SUBKEY_DEFAULT_MARGIN_LEFT */);
assert.equal((sut.element.children[0] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[4] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[5] as HTMLDivElement).style.marginTop, '5px');
Expand All @@ -108,7 +108,7 @@ describe('subkey menu width', () => {
);

// should be 2 rows with 3 keys each
assert.equal(sut.element.style.width, '315px' /* 3 * (100 + SUBKEY_DEFAULT_MARGIN_LEFT) */);
assert.equal(sut.element.style.width, '320px' /* 3 * (100 + SUBKEY_DEFAULT_MARGIN_LEFT) + SUBKEY_DEFAULT_MARGIN_LEFT*/);
assert.equal((sut.element.children[0] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[2] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[3] as HTMLDivElement).style.marginTop, '5px');
Expand All @@ -124,9 +124,9 @@ describe('subkey menu width', () => {
);

// should be 2 rows with 5 keys each
// First row: 4 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + 1 * (100 + SUBKEY_DEFAULT_MARGIN_LEFT) = 225
// Second row: 5 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) = 150
assert.equal(sut.element.style.width, '225px');
// First row: 4 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + 1 * (100 + SUBKEY_DEFAULT_MARGIN_LEFT) + SUBKEY_DEFAULT_MARGIN_LEFT = 230
// Second row: 5 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + SUBKEY_DEFAULT_MARGIN_LEFT = 155
assert.equal(sut.element.style.width, '230px');
assert.equal((sut.element.children[0] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[4] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[5] as HTMLDivElement).style.marginTop, '5px');
Expand All @@ -142,14 +142,47 @@ describe('subkey menu width', () => {
);

// should be 2 rows
// First row: 6 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + 1 * (100 + SUBKEY_DEFAULT_MARGIN_LEFT) = 285
// Second row: 2 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) = 60
assert.equal(sut.element.style.width, '285px');
// First row: 6 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + 1 * (100 + SUBKEY_DEFAULT_MARGIN_LEFT) + SUBKEY_DEFAULT_MARGIN_LEFT = 290
// Second row: 2 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + SUBKEY_DEFAULT_MARGIN_LEFT = 65
assert.equal(sut.element.style.width, '290px');
assert.equal((sut.element.children[0] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[6] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[7] as HTMLDivElement).style.marginTop, '5px');
});

it('3 subkeys same widths fitting in row', () => {
const sut = new SubkeyPopup(
mockSource(),
sinon.stub(),
mockVisualKeyboard(95 /*px*/),
mockKeys(25, 30, [DEFAULT_KEY, DEFAULT_KEY, DEFAULT_KEY]),
DEFAULT_GESTURE_PARAMS
);

// should be 1 row
assert.equal(sut.element.style.width, '95px');
assert.equal((sut.element.children[0] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[2] as HTMLDivElement).style.marginTop, '');
});

it('3 subkeys same widths not fitting in row', () => {
const sut = new SubkeyPopup(
mockSource(),
sinon.stub(),
mockVisualKeyboard(94 /*px*/),
mockKeys(25, 30, [DEFAULT_KEY, DEFAULT_KEY, DEFAULT_KEY]),
DEFAULT_GESTURE_PARAMS
);

// should be 2 rows
// First row: 2 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + SUBKEY_DEFAULT_MARGIN_LEFT = 65
// Second row: 1 * (25 + SUBKEY_DEFAULT_MARGIN_LEFT) + SUBKEY_DEFAULT_MARGIN_LEFT = 35
assert.equal(sut.element.style.width, '65px');
assert.equal((sut.element.children[0] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[1] as HTMLDivElement).style.marginTop, '');
assert.equal((sut.element.children[2] as HTMLDivElement).style.marginTop, '5px');
});

it('One monster subkey that does not fit in a row', () => {
const sut = new SubkeyPopup(
mockSource(),
Expand All @@ -159,7 +192,7 @@ describe('subkey menu width', () => {
DEFAULT_GESTURE_PARAMS
);

assert.equal(sut.element.style.width, '145px');
assert.equal(sut.element.style.width, '150px');
assert.equal((sut.element.children[0] as HTMLDivElement).style.marginTop, '');
});

Expand Down

0 comments on commit eb9b34c

Please sign in to comment.