Commit cd6a1288 authored by Kyle Keating's avatar Kyle Keating Committed by Kyle Keating

Improve Adder location for touch screen devices

- Ensure that the adder location is always below the highlighted selection on a touch devices so that it does not compete with the copy/paste bar.
- Add a little extra margin (10px) to the adder on touch devices to give room for the selection handles
- Add helper method `isTouchDevice` for touch devices
- Add comment in isSelectionBackwards to explain that iOS does not have the same selection object as non-iOS devices
- Add 1 missing test coverage case in adder.js
parent d0001eaf
import { createElement, render } from 'preact'; import { createElement, render } from 'preact';
import AdderToolbar from './components/adder-toolbar'; import AdderToolbar from './components/adder-toolbar';
import { isTouchDevice } from '../shared/user-agent';
import { createShadowRoot } from './util/shadow-root'; import { createShadowRoot } from './util/shadow-root';
/** /**
...@@ -180,9 +181,12 @@ export class Adder { ...@@ -180,9 +181,12 @@ export class Adder {
// Set the initial arrow direction based on whether the selection was made // Set the initial arrow direction based on whether the selection was made
// forwards/upwards or downwards/backwards. // forwards/upwards or downwards/backwards.
/** @type {ArrowDirection} */ let arrowDirection; /** @type {ArrowDirection} */ let arrowDirection;
if (isRTLselection) { if (isRTLselection && !isTouchDevice()) {
arrowDirection = ARROW_POINTING_DOWN; arrowDirection = ARROW_POINTING_DOWN;
} else { } else {
// Render the adder below the selection for touch devices due to competing
// space with the native copy/paste bar that typical (not always) renders above
// the selection.
arrowDirection = ARROW_POINTING_UP; arrowDirection = ARROW_POINTING_UP;
} }
let top; let top;
...@@ -192,6 +196,9 @@ export class Adder { ...@@ -192,6 +196,9 @@ export class Adder {
// and close to the end. // and close to the end.
const hMargin = Math.min(ARROW_H_MARGIN, selectionRect.width); const hMargin = Math.min(ARROW_H_MARGIN, selectionRect.width);
const adderWidth = this._width(); const adderWidth = this._width();
// Render the adder a little lower on touch devices to provide room for the native
// selection handles so that the interactions with selection don't compete with the adder.
const touchScreenOffset = isTouchDevice() ? 10 : 0;
const adderHeight = this._height(); const adderHeight = this._height();
if (isRTLselection) { if (isRTLselection) {
left = selectionRect.left - adderWidth / 2 + hMargin; left = selectionRect.left - adderWidth / 2 + hMargin;
...@@ -212,7 +219,11 @@ export class Adder { ...@@ -212,7 +219,11 @@ export class Adder {
} }
if (arrowDirection === ARROW_POINTING_UP) { if (arrowDirection === ARROW_POINTING_UP) {
top = selectionRect.top + selectionRect.height + ARROW_HEIGHT; top =
selectionRect.top +
selectionRect.height +
ARROW_HEIGHT +
touchScreenOffset;
} else { } else {
top = selectionRect.top - adderHeight - ARROW_HEIGHT; top = selectionRect.top - adderHeight - ARROW_HEIGHT;
} }
......
...@@ -10,6 +10,8 @@ export function isSelectionBackwards(selection) { ...@@ -10,6 +10,8 @@ export function isSelectionBackwards(selection) {
} }
const range = selection.getRangeAt(0); const range = selection.getRangeAt(0);
// Does not work correctly on iOS when selecting nodes backwards.
// https://bugs.webkit.org/show_bug.cgi?id=220523
return range.startContainer === selection.focusNode; return range.startContainer === selection.focusNode;
} }
......
...@@ -2,7 +2,12 @@ import { act } from 'preact/test-utils'; ...@@ -2,7 +2,12 @@ import { act } from 'preact/test-utils';
import { createElement } from 'preact'; import { createElement } from 'preact';
import { mount } from 'enzyme'; import { mount } from 'enzyme';
import { Adder, ARROW_POINTING_UP, ARROW_POINTING_DOWN } from '../adder'; import {
Adder,
ARROW_POINTING_UP,
ARROW_POINTING_DOWN,
$imports,
} from '../adder';
function rect(left, top, width, height) { function rect(left, top, width, height) {
return { left, top, width, height }; return { left, top, width, height };
...@@ -50,6 +55,7 @@ describe('Adder', () => { ...@@ -50,6 +55,7 @@ describe('Adder', () => {
afterEach(() => { afterEach(() => {
adderCtrl.hide(); adderCtrl.hide();
adderEl.remove(); adderEl.remove();
$imports.$restore();
}); });
function windowSize() { function windowSize() {
...@@ -199,6 +205,12 @@ describe('Adder', () => { ...@@ -199,6 +205,12 @@ describe('Adder', () => {
assert.equal(target.arrowDirection, ARROW_POINTING_UP); assert.equal(target.arrowDirection, ARROW_POINTING_UP);
}); });
it('does not position the adder above the top of the viewport even when selection is backwards', () => {
const target = adderCtrl._calculateTarget(rect(100, -100, 100, 20), true);
assert.isAtLeast(target.top, 0);
assert.equal(target.arrowDirection, ARROW_POINTING_UP);
});
it('does not position the adder below the bottom of the viewport', () => { it('does not position the adder below the bottom of the viewport', () => {
const viewSize = windowSize(); const viewSize = windowSize();
const target = adderCtrl._calculateTarget( const target = adderCtrl._calculateTarget(
...@@ -221,6 +233,22 @@ describe('Adder', () => { ...@@ -221,6 +233,22 @@ describe('Adder', () => {
const target = adderCtrl._calculateTarget(rect(-100, 100, 10, 10), false); const target = adderCtrl._calculateTarget(rect(-100, 100, 10, 10), false);
assert.isAtLeast(target.left, 0); assert.isAtLeast(target.left, 0);
}); });
context('touch device', () => {
it('positions the adder below the selection even if the selection is backwards', () => {
$imports.$mock({
'../shared/user-agent': {
isTouchDevice: sinon.stub().returns(true),
},
});
const target = adderCtrl._calculateTarget(
rect(100, 200, 100, 20),
true
);
assert.isAbove(target.top, 220);
assert.equal(target.arrowDirection, ARROW_POINTING_UP);
});
});
}); });
describe('adder Z index', () => { describe('adder Z index', () => {
......
import { isMacOS, isIOS } from '../user-agent'; import { isMacOS, isIOS, isTouchDevice } from '../user-agent';
describe('shared/user-agent', () => { describe('shared/user-agent', () => {
describe('isMacOS', () => { describe('isMacOS', () => {
...@@ -19,6 +19,23 @@ describe('shared/user-agent', () => { ...@@ -19,6 +19,23 @@ describe('shared/user-agent', () => {
}); });
}); });
describe('isTouchDevice', () => {
let matchMedia;
beforeEach(() => {
matchMedia = sinon.spy(window, 'matchMedia');
});
afterEach(() => {
window.matchMedia.restore();
});
it('calls `window.matchMedia` with the query string "(pointer: coarse)"', () => {
isTouchDevice(window);
assert.calledWith(matchMedia, '(pointer: coarse)');
});
});
describe('isIOS', () => { describe('isIOS', () => {
it('returns true when the user agent is an iOS', () => { it('returns true when the user agent is an iOS', () => {
assert.isBoolean(isIOS()); // Test to check default parameters assert.isBoolean(isIOS()); // Test to check default parameters
......
...@@ -35,3 +35,14 @@ export const isIOS = ( ...@@ -35,3 +35,14 @@ export const isIOS = (
(_navigator.userAgent.includes('Mac') && _ontouchend) (_navigator.userAgent.includes('Mac') && _ontouchend)
); );
}; };
/**
* Returns true when the device is a touch device such
* as android or iOS.
* https://developer.mozilla.org/en-US/docs/Web/CSS/@media/pointer#browser_compatibility
*
* @param _window {Window} - Test seam
*/
export const isTouchDevice = (_window = window) => {
return _window.matchMedia('(pointer: coarse)').matches;
};
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment