Skip to content

Commit

Permalink
[uiActions] Improve context menu keyboard support (#70705) (#71435)
Browse files Browse the repository at this point in the history
* Improves position resolution logic by also tracking last clicked element.
* Adds ownFocus prop, so can pick menu item with keyboard.
* Also track if target element was removed from DOM. In that case tries to use previous element. won't work all the time, but works nicely in case context menu trigger by item in other context menu.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
Dosant and elasticmachine authored Jul 13, 2020
1 parent 48c8f45 commit 7bcc84d
Show file tree
Hide file tree
Showing 2 changed files with 161 additions and 40 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { createInteractionPositionTracker } from './open_context_menu';
import { fireEvent } from '@testing-library/dom';

let targetEl: Element;
const top = 100;
const left = 100;
const right = 200;
const bottom = 200;
beforeEach(() => {
targetEl = document.createElement('div');
jest.spyOn(targetEl, 'getBoundingClientRect').mockImplementation(() => ({
top,
left,
right,
bottom,
width: right - left,
height: bottom - top,
x: left,
y: top,
toJSON: () => {},
}));
document.body.append(targetEl);
});
afterEach(() => {
targetEl.remove();
});

test('should use last clicked element position if mouse position is outside target element', () => {
const { resolveLastPosition } = createInteractionPositionTracker();

fireEvent.click(targetEl, { clientX: 0, clientY: 0 });
const { x, y } = resolveLastPosition();

expect(y).toBe(bottom);
expect(x).toBe(left + (right - left) / 2);
});

test('should use mouse position if mouse inside clicked element', () => {
const { resolveLastPosition } = createInteractionPositionTracker();

const mouseX = 150;
const mouseY = 150;
fireEvent.click(targetEl, { clientX: mouseX, clientY: mouseY });

const { x, y } = resolveLastPosition();

expect(y).toBe(mouseX);
expect(x).toBe(mouseY);
});

test('should use position of previous element, if latest element is no longer in DOM', () => {
const { resolveLastPosition } = createInteractionPositionTracker();

const detachedElement = document.createElement('div');
const spy = jest.spyOn(detachedElement, 'getBoundingClientRect');

fireEvent.click(targetEl);
fireEvent.click(detachedElement);

const { x, y } = resolveLastPosition();

expect(y).toBe(bottom);
expect(x).toBe(left + (right - left) / 2);
expect(spy).not.toBeCalled();
});
117 changes: 77 additions & 40 deletions src/plugins/ui_actions/public/context_menu/open_context_menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,86 @@ import ReactDOM from 'react-dom';
let activeSession: ContextMenuSession | null = null;

const CONTAINER_ID = 'contextMenu-container';
let initialized = false;

/**
* Tries to find best position for opening context menu using mousemove and click event
* Returned position is relative to document
*/
export function createInteractionPositionTracker() {
let lastMouseX = 0;
let lastMouseY = 0;
const lastClicks: Array<{ el?: Element; mouseX: number; mouseY: number }> = [];
const MAX_LAST_CLICKS = 10;

/**
* Track both `mouseup` and `click`
* `mouseup` is for clicks and brushes with mouse
* `click` is a fallback for keyboard interactions
*/
document.addEventListener('mouseup', onClick, true);
document.addEventListener('click', onClick, true);
document.addEventListener('mousemove', onMouseUpdate, { passive: true });
document.addEventListener('mouseenter', onMouseUpdate, { passive: true });
function onClick(event: MouseEvent) {
lastClicks.push({
el: event.target as Element,
mouseX: event.clientX,
mouseY: event.clientY,
});
if (lastClicks.length > MAX_LAST_CLICKS) {
lastClicks.shift();
}
}
function onMouseUpdate(event: MouseEvent) {
lastMouseX = event.clientX;
lastMouseY = event.clientY;
}

return {
resolveLastPosition: (): { x: number; y: number } => {
const lastClick = [...lastClicks]
.reverse()
.find(({ el }) => el && document.body.contains(el));
if (!lastClick) {
// fallback to last mouse position
return {
x: lastMouseX,
y: lastMouseY,
};
}

const { top, left, bottom, right } = lastClick.el!.getBoundingClientRect();

const mouseX = lastClick.mouseX;
const mouseY = lastClick.mouseY;

if (top <= mouseY && bottom >= mouseY && left <= mouseX && right >= mouseX) {
// click was inside target element
return {
x: mouseX,
y: mouseY,
};
} else {
// keyboard edge case. no cursor position. use target element position instead
return {
x: left + (right - left) / 2,
y: bottom,
};
}
},
};
}

const { resolveLastPosition } = createInteractionPositionTracker();
function getOrCreateContainerElement() {
let container = document.getElementById(CONTAINER_ID);
const y = getMouseY() + document.body.scrollTop;
let { x, y } = resolveLastPosition();
y = y + window.scrollY;
x = x + window.scrollX;

if (!container) {
container = document.createElement('div');
container.style.left = getMouseX() + 'px';
container.style.left = x + 'px';
container.style.top = y + 'px';
container.style.position = 'absolute';

Expand All @@ -44,38 +116,12 @@ function getOrCreateContainerElement() {
container.id = CONTAINER_ID;
document.body.appendChild(container);
} else {
container.style.left = getMouseX() + 'px';
container.style.left = x + 'px';
container.style.top = y + 'px';
}
return container;
}

let x: number = 0;
let y: number = 0;

function initialize() {
if (!initialized) {
document.addEventListener('mousemove', onMouseUpdate, false);
document.addEventListener('mouseenter', onMouseUpdate, false);
initialized = true;
}
}

function onMouseUpdate(e: any) {
x = e.pageX;
y = e.pageY;
}

function getMouseX() {
return x;
}

function getMouseY() {
return y;
}

initialize();

/**
* A FlyoutSession describes the session of one opened flyout panel. It offers
* methods to close the flyout panel again. If you open a flyout panel you should make
Expand All @@ -87,16 +133,6 @@ initialize();
* @extends EventEmitter
*/
class ContextMenuSession extends EventEmitter {
/**
* Binds the current flyout session to an Angular scope, meaning this flyout
* session will be closed as soon as the Angular scope gets destroyed.
* @param {object} scope - An angular scope object to bind to.
*/
public bindToAngularScope(scope: ng.IScope): void {
const removeWatch = scope.$on('$destroy', () => this.close());
this.on('closed', () => removeWatch());
}

/**
* Closes the opened flyout as long as it's still the open one.
* If this is not the active session anymore, this method won't do anything.
Expand Down Expand Up @@ -151,6 +187,7 @@ export function openContextMenu(
panelPaddingSize="none"
anchorPosition="downRight"
withTitle
ownFocus={true}
>
<EuiContextMenu
initialPanelId="mainMenu"
Expand Down

0 comments on commit 7bcc84d

Please sign in to comment.