Skip to content

Commit

Permalink
fix(synthetic): consider ancestors when determining click-focusability (
Browse files Browse the repository at this point in the history
#1383)

* test: reproduce bug described by issue #1382

* fix: consider ancestors when determining focusability
  • Loading branch information
ekashida authored Jun 28, 2019
1 parent c270594 commit 5d4dc4d
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 10 deletions.
34 changes: 24 additions & 10 deletions packages/@lwc/synthetic-shadow/src/faux-shadow/focus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
isNull,
isUndefined,
toString,
isTrue,
} from '../shared/language';
import {
DocumentPrototypeActiveElement,
Expand Down Expand Up @@ -104,14 +105,23 @@ const focusableTagNames = {
// This function based on https://allyjs.io/data-tables/focusable.html
// It won't catch everything, but should be good enough
// There are a lot of edge cases here that we can't realistically handle
function isFocusable(element: HTMLElement): boolean {
const tagName = tagNameGetter.call(element);
return (
function isFocusable(element: HTMLElement, hostElement: HTMLElement): boolean {
const focusable =
isVisible(element) &&
(hasFocusableTabIndex(element) ||
hasAttribute.call(element, 'contenteditable') ||
hasOwnProperty.call(focusableTagNames, tagName))
);
hasOwnProperty.call(focusableTagNames, tagNameGetter.call(element)));

if (isTrue(focusable)) {
return true;
}

const nextElement =
element.parentNode === element.getRootNode()
? (element.parentNode as ShadowRoot).host
: element.parentElement!;

return nextElement !== hostElement && isFocusable(nextElement as HTMLElement, hostElement);
}

interface QuerySegments {
Expand Down Expand Up @@ -325,10 +335,15 @@ function getNextTabbable(tabbables: HTMLElement[], relatedTarget: EventTarget):
return null;
}

function willTriggerFocusInEvent(target: HTMLElement): boolean {
const doc = getOwnerDocument(target);
function willTriggerFocusInEvent(event: MouseEvent): boolean {
const target = eventTargetGetter.call(event);
const doc = getOwnerDocument(target as Node);
const activeElement = DocumentPrototypeActiveElement.call(doc);
const isTargetActiveElement = target === activeElement;
return (
target !== DocumentPrototypeActiveElement.call(doc) && isFocusable(target) // if the element is currently active, it will not fire a focusin event
// if the element is currently active, it will not fire a focusin event
!isTargetActiveElement &&
isFocusable(target as HTMLElement, eventCurrentTargetGetter.call(event) as HTMLElement)
);
}

Expand Down Expand Up @@ -361,11 +376,10 @@ function exitMouseDownState(event) {
}

function handleFocusMouseDown(evt) {
const target = eventTargetGetter.call(evt) as HTMLElement;
// If we are mouse down in an element that can be focused
// and the currentTarget's activeElement is not element we are mouse-ing down in
// We can bail out and let the browser do its thing.
if (willTriggerFocusInEvent(target)) {
if (willTriggerFocusInEvent(evt)) {
const currentTarget = eventCurrentTargetGetter.call(evt);
// Enter the temporary state where we disable the keyboard focusin handler when we click into the shadow.
addEventListener.call(currentTarget, 'focusin', enterMouseDownState, true);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright (c) 2018, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
const assert = require('assert');

const URL = 'http://localhost:4567/delegates-focus-click-target-natively-non-focusable';

describe('when the click target is natively non-focusable', () => {
beforeEach(() => {
browser.url(URL);
});

it('should apply focus to natively focusable parent (button) when click target is custom element', () => {
const input = browser.execute(function() {
return document
.querySelector('integration-delegates-focus-click-target-natively-non-focusable')
.shadowRoot.querySelector('.head');
});
input.click();

// Click on the custom element wrapped by the button
const child = browser.execute(function() {
return document
.querySelector('integration-delegates-focus-click-target-natively-non-focusable')
.shadowRoot.querySelector('integration-parent')
.shadowRoot.querySelector('button > integration-child');
});
child.click();

const className = browser.execute(function() {
return document
.activeElement.shadowRoot.activeElement.shadowRoot.activeElement.className;
}).value;

// The invalid behavior described in issue #1382 causes focus to land on input.tail
assert.equal(className, 'integration-child-button');
});

it('should apply focus to natively focusable parent (button) when click target is span element', () => {
const input = browser.execute(function() {
return document
.querySelector('integration-delegates-focus-click-target-natively-non-focusable')
.shadowRoot.querySelector('.head');
});
input.click();

// Click on the span wrapped by the button
const span = browser.execute(function() {
return document
.querySelector('integration-delegates-focus-click-target-natively-non-focusable')
.shadowRoot.querySelector('integration-parent')
.shadowRoot.querySelector('button > span');
});
span.click();

const className = browser.execute(function() {
return document
.activeElement.shadowRoot.activeElement.shadowRoot.activeElement.className;
}).value;

// The invalid behavior described in issue #1382 causes focus to land on input.tail
assert.equal(className, 'span-button');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
<div>integration-child content</div>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { LightningElement } from 'lwc';

export default class Child extends LightningElement {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<template>
<input class="head" placeholder="head">
<integration-parent tabindex="-1"></integration-parent>
<input class="tail" placeholder="tail">
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { LightningElement } from 'lwc';

export default class Container extends LightningElement {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:focus {
border-color: red;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<template>
<button class="integration-child-button">
<integration-child></integration-child>
</button>
<button class="span-button">
<span>span content</span>
</button>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { LightningElement } from 'lwc';

export default class Parent extends LightningElement {
static delegatesFocus = true;
}

0 comments on commit 5d4dc4d

Please sign in to comment.