Skip to content

Commit

Permalink
fix(textfield): don't show focus indicator when focused on icon
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 564527586
  • Loading branch information
asyncLiz authored and copybara-github committed Sep 11, 2023
1 parent 68a078b commit 61c8f6d
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 44 deletions.
3 changes: 2 additions & 1 deletion field/internal/field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ export class Field extends LitElement implements SurfacePositionTarget {

protected override update(props: PropertyValues<Field>) {
// Client-side property updates
const isDisabledChanging = props.has('disabled') && props.get('disabled') !== undefined;
const isDisabledChanging =
props.has('disabled') && props.get('disabled') !== undefined;
if (isDisabledChanging) {
this.disableTransitions = true;
}
Expand Down
13 changes: 0 additions & 13 deletions textfield/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,6 @@ export class TextFieldHarness extends Harness<TextField> {

this.valueBeforeChange = textField.value;
super.simulatePointerFocus(input);

const prevFocus = input.focus;
if (prevFocus === HTMLElement.prototype.focus) {
// Swap to a no-op if nobody is spying on this method so that we don't
// generate side-effects when we actually call focus().
input.focus = () => {};
}

// Call focus() as a side-effect since delegatesFocus won't be triggered
// with this simulation. Replace input.focus() with a no-op if needed to
// avoid actually calling focus.
textField.focus();
input.focus = prevFocus;
}

protected simulateInput(
Expand Down
33 changes: 5 additions & 28 deletions textfield/internal/text-field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {html, isServer, LitElement, nothing, PropertyValues} from 'lit';
import {html, LitElement, nothing, PropertyValues} from 'lit';
import {property, query, queryAssignedElements, state} from 'lit/decorators.js';
import {classMap} from 'lit/directives/class-map.js';
import {live} from 'lit/directives/live.js';
Expand Down Expand Up @@ -350,15 +350,6 @@ export abstract class TextField extends LitElement {
private readonly internals =
(this as HTMLElement /* needed for closure */).attachInternals();

constructor() {
super();
if (!isServer) {
this.addEventListener('click', this.focus);
this.addEventListener('focusin', this.handleFocusin);
this.addEventListener('focusout', this.handleFocusout);
}
}

/**
* Checks the text field's native validation and returns whether or not the
* element is valid.
Expand All @@ -374,19 +365,6 @@ export abstract class TextField extends LitElement {
return this.internals.checkValidity();
}

/**
* Focuses the text field's input text.
*/
override focus() {
if (this.disabled || this.matches(':focus-within')) {
// Don't shift focus from an element within the text field, like an icon
// button, to the input when focus is requested.
return;
}

super.focus();
}

/**
* Checks the text field's native validation and returns whether or not the
* element is valid.
Expand Down Expand Up @@ -640,6 +618,8 @@ export abstract class TextField extends LitElement {
rows=${this.rows}
.value=${live(this.value)}
@change=${this.handleChange}
@focusin=${this.handleFocusin}
@focusout=${this.handleFocusout}
@input=${this.handleInput}
@select=${this.redispatchEvent}
></textarea>
Expand Down Expand Up @@ -677,6 +657,8 @@ export abstract class TextField extends LitElement {
type=${this.type}
.value=${live(this.value)}
@change=${this.redispatchEvent}
@focusin=${this.handleFocusin}
@focusout=${this.handleFocusout}
@input=${this.handleInput}
@select=${this.redispatchEvent}
>
Expand Down Expand Up @@ -715,11 +697,6 @@ export abstract class TextField extends LitElement {
}

private handleFocusout() {
if (this.matches(':focus-within')) {
// Changing focus to another child within the text field, like a button
return;
}

this.focused = false;
}

Expand Down
5 changes: 3 additions & 2 deletions textfield/internal/text-field_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('TextField', () => {
expect(input.matches(':focus')).withContext('is input:focus').toBeTrue();
});

it('should focus the input when elements inside text field are clicked',
it('should NOT focus the input when elements inside text field are clicked',
async () => {
const {harness, input} = await setupTest();
// Add a trailing icon button to click on
Expand All @@ -86,12 +86,13 @@ describe('TextField', () => {

expect(input.matches(':focus'))
.withContext('is input:focus')
.toBeTrue();
.toBeFalse();
});

it('should not focus the input when disabled', async () => {
const {harness, input} = await setupTest();
harness.element.disabled = true;
await env.waitForStability();

harness.element.focus();

Expand Down

0 comments on commit 61c8f6d

Please sign in to comment.