Skip to content

Commit

Permalink
fix: use click event for opening sub-menus on touch devices (#3322)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tomivirkki
tomivirkki committed Jan 24, 2022
1 parent aa22bc1 commit ae36306
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 24 deletions.
6 changes: 5 additions & 1 deletion packages/context-menu/src/vaadin-contextmenu-items-mixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { flush } from '@polymer/polymer/lib/utils/flush.js';
import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
import { Item } from '@vaadin/item/src/vaadin-item.js';
import { ListBox } from '@vaadin/list-box/src/vaadin-list-box.js';

Expand Down Expand Up @@ -361,7 +362,10 @@ export const ItemsMixin = (superClass) =>
}
};

menu.$.overlay.addEventListener('mouseover', openSubMenu);
// Open a submenu on click event when a touch device is used.
// On desktop, a submenu opens on hover.
menu.$.overlay.addEventListener(isTouch ? 'click' : 'mouseover', openSubMenu);

menu.$.overlay.addEventListener('keydown', (e) => {
const isRTL = this.__isRTL;
const shouldOpenSubMenu =
Expand Down
24 changes: 13 additions & 11 deletions packages/context-menu/test/items.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import {
escKeyDown,
fire,
fixtureSync,
isChrome,
isIOS,
nextFrame,
nextRender,
spaceKeyDown
Expand All @@ -19,6 +17,9 @@ import '@vaadin/item/vaadin-item.js';
import '@vaadin/list-box/vaadin-list-box.js';
import './not-animated-styles.js';
import '../vaadin-context-menu.js';
import { isTouch } from '@vaadin/component-base/src/browser-utils.js';

const menuOpenEvent = isTouch ? 'click' : 'mouseover';

describe('items', () => {
let rootMenu, subMenu, target;
Expand All @@ -35,7 +36,7 @@ describe('items', () => {
overlay.__openingHandler && overlay.__openingHandler();
}
const { right, bottom } = openTarget.getBoundingClientRect();
fire(openTarget, 'mouseover', { x: right, y: bottom });
fire(openTarget, menuOpenEvent, { x: right, y: bottom });
};

const getSubMenu = (menu = rootMenu) => {
Expand Down Expand Up @@ -66,7 +67,7 @@ describe('items', () => {
<button id="target"></button>
</vaadin-context-menu>
`);
rootMenu.openOn = 'mouseover';
rootMenu.openOn = menuOpenEvent;
target = rootMenu.firstElementChild;
rootMenu.items = [
{
Expand Down Expand Up @@ -117,13 +118,13 @@ describe('items', () => {
expect(spy.called).to.be.false;
});

(isIOS ? it.skip : it)('should open the subMenu on the right side', async () => {
(isTouch ? it.skip : it)('should open the subMenu on the right side', async () => {
const rootItemRect = menuComponents()[0].getBoundingClientRect();
const subItemRect = menuComponents(subMenu)[0].getBoundingClientRect();
expect(subItemRect.left).to.be.above(rootItemRect.right);
});

(isIOS ? it.skip : it)('should open the subMenu on the left side', () => {
(isTouch ? it.skip : it)('should open the subMenu on the left side', () => {
subMenu.close();
let rootItemRect = menuComponents()[0].getBoundingClientRect();
rootMenu.$.overlay.style.left = window.innerWidth - rootItemRect.width * 1.5 + 'px';
Expand All @@ -133,7 +134,7 @@ describe('items', () => {
expect(subItemRect.right).to.be.below(rootItemRect.left);
});

(isIOS ? it.skip : it)('should open the subMenu on the top if root menu is bottom-aligned', async () => {
(isTouch ? it.skip : it)('should open the subMenu on the top if root menu is bottom-aligned', async () => {
subMenu.close();
rootMenu.$.overlay.style.removeProperty('top');
rootMenu.$.overlay.style.bottom = '0px';
Expand All @@ -145,7 +146,7 @@ describe('items', () => {
expect(subMenuRect.bottom).to.be.below(rootMenuRect.bottom);
});

(isIOS ? it.skip : it)('should open the subMenu on the left if root menu is end-aligned', async () => {
(isTouch ? it.skip : it)('should open the subMenu on the left if root menu is end-aligned', async () => {
subMenu.close();
await nextRender(subMenu);
const rootItem = menuComponents()[0];
Expand Down Expand Up @@ -250,7 +251,7 @@ describe('items', () => {
expect(subMenu.opened).to.be.false;
});

it('should focus closed parent item when hovering on non-parent item', () => {
(isTouch ? it.skip : it)('should focus closed parent item when hovering on non-parent item', () => {
const parent = menuComponents()[0];
const nonParent = menuComponents()[1];
const focusSpy = sinon.spy(parent, 'focus');
Expand Down Expand Up @@ -349,6 +350,7 @@ describe('items', () => {
open(menuComponents()[0]);
await nextRender(subMenu);
expect(subMenu.opened).to.be.true;
await nextRender(subMenu);
expect(menuComponents(subMenu)[0].hasAttribute('focused')).to.be.false;
});

Expand Down Expand Up @@ -432,7 +434,7 @@ describe('items', () => {
expect(menuComponents()[0].getAttribute('aria-expanded')).to.equal('false');
});

(isChrome ? describe : describe.skip)('scrolling', () => {
(isTouch ? describe.skip : describe)('scrolling', () => {
let rootOverlay, subOverlay1, subOverlay2, scrollElm;

beforeEach(async () => {
Expand Down Expand Up @@ -502,7 +504,7 @@ describe('items', () => {
<button id="target"></button>
</vaadin-context-menu>
`);
rootMenu.openOn = 'mouseover';
rootMenu.openOn = menuOpenEvent;
target = rootMenu.firstElementChild;
rootMenu.items = [
{
Expand Down
32 changes: 20 additions & 12 deletions packages/menu-bar/test/sub-menu.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@ import {
import sinon from 'sinon';
import './not-animated-styles.js';
import '../vaadin-menu-bar.js';
import { setCancelSyntheticClickEvents } from '@polymer/polymer/lib/utils/settings.js';
import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
import { onceOpened } from './helpers.js';

setCancelSyntheticClickEvents(false);

const menuOpenEvent = isTouch ? 'click' : 'mouseover';

describe('sub-menu', () => {
let menu, buttons, subMenu, item;

Expand Down Expand Up @@ -436,6 +442,8 @@ describe('sub-menu', () => {
describe('open on hover', () => {
let menu, buttons, subMenu;

const openOnHoverEvent = 'mouseover';

beforeEach(async () => {
menu = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>');
menu.items = [
Expand Down Expand Up @@ -464,11 +472,11 @@ describe('open on hover', () => {
it('should set pointer events to `auto` when opened and remove when closed', async () => {
expect(menu.style.pointerEvents).to.be.empty;

buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[0].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
expect(menu.style.pointerEvents).to.equal('auto');

buttons[2].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[2].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
expect(menu.style.pointerEvents).to.equal('auto');

Expand All @@ -478,16 +486,16 @@ describe('open on hover', () => {
});

it('should open sub-menu on mouseover on button with nested items', async () => {
buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[0].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
expect(subMenu.opened).to.be.true;
expect(subMenu.listenOn).to.equal(buttons[0]);
});

it('should close open sub-menu on mouseover on button without nested items', async () => {
buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[0].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
buttons[1].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[1].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
expect(subMenu.opened).to.be.false;
});
Expand All @@ -496,7 +504,7 @@ describe('open on hover', () => {
menu.openOnHover = false;
buttons[0].click();
await nextRender(subMenu);
buttons[2].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[2].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
expect(subMenu.opened).to.be.true;
expect(subMenu.listenOn).to.equal(buttons[2]);
Expand All @@ -505,14 +513,14 @@ describe('open on hover', () => {
it('should not select value of button without nested items', async () => {
const spy = sinon.spy();
menu.addEventListener('item-selected', spy);
buttons[1].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[1].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
expect(spy.called).to.be.false;
});

it('should not close sub-menu on expanded button mouseover', async () => {
buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[0].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[0].dispatchEvent(new CustomEvent(openOnHoverEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
expect(subMenu.opened).to.be.true;
});
Expand Down Expand Up @@ -604,7 +612,7 @@ describe('theme attribute', () => {

// open submenu
menu.openOnHover = true;
buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[0].dispatchEvent(new CustomEvent(menuOpenEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);
});

Expand All @@ -631,7 +639,7 @@ describe('theme attribute', () => {

subMenu.close();
menu.removeAttribute('theme');
buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
buttons[0].dispatchEvent(new CustomEvent(menuOpenEvent, { bubbles: true, composed: true }));
await nextRender(subMenu);

items = subMenu.$.overlay.querySelectorAll('vaadin-context-menu-item');
Expand All @@ -651,7 +659,7 @@ describe('touch', () => {
const overlay = menu.$.overlay;
overlay.__openingHandler && overlay.__openingHandler();
}
openTarget.dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
openTarget.dispatchEvent(new CustomEvent(menuOpenEvent, { bubbles: true, composed: true }));
};

beforeEach(async () => {
Expand Down

0 comments on commit ae36306

Please sign in to comment.