Skip to content

Commit

Permalink
fix: do not delegate focus when clicking on non-focusable child (#3565)…
Browse files Browse the repository at this point in the history
… (#3567)

* fix: do not delegate focus when clicking on non-focusable child

* reset mouse state after tests

Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
  • Loading branch information
vaadin-bot and sissbruecker committed Mar 15, 2022
1 parent c1c2da0 commit c68d8fc
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
4 changes: 3 additions & 1 deletion packages/field-base/src/shadow-focus-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ export const ShadowFocusMixin = (superClass) =>
const path = event.composedPath();

// When focus moves from outside and not with Shift + Tab, delegate it to focusElement.
if (path[0] === this && !this.contains(event.relatedTarget) && !this._isShiftTabbing) {
// This should only move focus when using keyboard navigation, for clicks we don't want to interfere,
// for example when the user tries to select some text
if (this._keyboardActive && path[0] === this && !this.contains(event.relatedTarget) && !this._isShiftTabbing) {
this.focusElement.focus();
return true;
}
Expand Down
31 changes: 29 additions & 2 deletions packages/field-base/test/shadow-focus-mixin.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from '@esm-bundle/chai';
import { aTimeout, fixtureSync, focusin, focusout, keyboardEventFor, keyDownOn } from '@vaadin/testing-helpers';
import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands';
import sinon from 'sinon';
import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
import { ShadowFocusMixin } from '../src/shadow-focus-mixin.js';
Expand All @@ -11,6 +12,7 @@ customElements.define(
return html`
<input id="input" />
<input id="secondInput" />
<slot></slot>
`;
}

Expand Down Expand Up @@ -47,7 +49,7 @@ describe('shadow-focus-mixin', () => {
expect(focusElement.getAttribute('tabindex')).to.be.equal('1');
});

it('should se tabindex to 0 by default', () => {
it('should set tabindex to 0 by default', () => {
expect(customElement.getAttribute('tabindex')).to.be.equal('0');
});

Expand Down Expand Up @@ -186,6 +188,10 @@ describe('shadow-focus-mixin', () => {
});

describe('focus', () => {
afterEach(async () => {
await resetMouse();
});

it('should call focus on focusElement', () => {
const spy = sinon.spy(focusElement, 'focus');
customElement.focus();
Expand All @@ -202,7 +208,28 @@ describe('shadow-focus-mixin', () => {
expect(customElement.hasAttribute('focused')).to.be.true;
});

it('should not focus if the focus is not received from outside', () => {
it('should delegate focus if the focus is received from outside using keyboard navigation', async () => {
const spy = sinon.spy(focusElement, 'focus');
await sendKeys({ press: 'Tab' });
expect(spy.called).to.be.true;
});

it('should not delegate focus when clicking on non-focusable child', async () => {
const span = document.createElement('span');
span.textContent = 'test';
customElement.appendChild(span);

const spy = sinon.spy(focusElement, 'focus');
// Click center of span child element
const rect = span.getBoundingClientRect();
const middleX = Math.floor(rect.x + rect.width / 2);
const middleY = Math.floor(rect.y + rect.height / 2);
await sendMouse({ type: 'click', position: [middleX, middleY] });
// Clicking on some text content should not move focus
expect(spy.called).to.be.false;
});

it('should not delegate focus if the focus is not received from outside', () => {
const child = document.createElement('div');
customElement.appendChild(child);

Expand Down

0 comments on commit c68d8fc

Please sign in to comment.