Skip to content

Commit

Permalink
fix(textfield)!: remove defaultValue
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: Explicit "defaultValue" has been removed. Set the 'value' attribute to communicate a default value for resetting (similar to native <input>)

PiperOrigin-RevId: 532095818
  • Loading branch information
@asyncLiz @copybara-github
asyncLiz authored and copybara-github committed May 15, 2023
1 parent 4ddeee1 commit 2317c5a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 84 deletions.
62 changes: 10 additions & 52 deletions textfield/lib/text-field.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,8 @@ export abstract class TextField extends LitElement {
@property({type: Boolean, reflect: true}) required = false;
/**
* The current value of the text field. It is always a string.
*
* This is equal to `defaultValue` before user input.
*/
@property() value = '';
/**
* The default value of the text field. Before user input, changing the
* default value will update `value` as well.
*
* When the text field is reset, its `value` will be set to this default
* value.
*/
@property() defaultValue = '';
/**
* An optional prefix to display before the input value.
*/
Expand Down Expand Up @@ -267,19 +257,6 @@ export abstract class TextField extends LitElement {
* to screen readers.
*/
@state() private refreshErrorAlert = false;
/**
* Returns true when the text field's `value` property has been changed from
* it's initial value.
*
* Setting `value` should always overwrite `defaultValue`, even when `value`
* is an empty string. This flag ensures that behavior.
*/
@state() private valueHasChanged = false;
/**
* Whether or not to ignore the next `value` change when computing
* `valueHasChanged`.
*/
private ignoreNextValueChange = false;
/**
* Whether or not a native error has been reported via `reportValidity()`.
*/
Expand Down Expand Up @@ -462,27 +439,20 @@ export abstract class TextField extends LitElement {
*/
reset() {
this.dirty = false;
this.valueHasChanged = false;
this.ignoreNextValueChange = true;
this.value = this.defaultValue;
this.value = this.getAttribute('value') ?? '';
this.nativeError = false;
this.nativeErrorText = '';
}

protected override update(changedProperties: PropertyValues) {
// Consider a value change anything that is not the initial empty string
// value.
const valueHasChanged = changedProperties.has('value') &&
changedProperties.get('value') !== undefined;
if (valueHasChanged && !this.ignoreNextValueChange) {
this.valueHasChanged = true;
}

if (this.ignoreNextValueChange) {
this.ignoreNextValueChange = false;
override attributeChangedCallback(
attribute: string, newValue: string|null, oldValue: string|null) {
if (attribute === 'value' && this.dirty) {
// After user input, changing the value attribute no longer updates the
// text field's value (until reset). This matches native <input> behavior.
return;
}

super.update(changedProperties);
super.attributeChangedCallback(attribute, newValue, oldValue);
}

protected override render() {
Expand All @@ -505,9 +475,6 @@ export abstract class TextField extends LitElement {
// value to change without dispatching an event, re-sync it.
const value = this.getInput().value;
if (this.value !== value) {
// Don't consider these updates (such as setting `defaultValue`) as
// the developer directly changing the `value`.
this.ignoreNextValueChange = true;
// Note this is typically inefficient in updated() since it schedules
// another update. However, it is needed for the <input> to fully render
// before checking its value.
Expand Down Expand Up @@ -536,7 +503,7 @@ export abstract class TextField extends LitElement {
?hasEnd=${this.hasTrailingIcon}
?hasStart=${this.hasLeadingIcon}
.label=${this.label}
?populated=${!!this.getInputValue()}
?populated=${!!this.value}
?required=${this.required}
>
${this.renderLeadingIcon()}
Expand Down Expand Up @@ -588,22 +555,13 @@ export abstract class TextField extends LitElement {
?required=${this.required}
step=${(this.step || nothing) as unknown as number}
type=${this.type}
.value=${live(this.getInputValue())}
.value=${live(this.value)}
@change=${this.redispatchEvent}
@input=${this.handleInput}
@select=${this.redispatchEvent}
>`;
}

private getInputValue() {
const alwaysShowValue = this.dirty || this.valueHasChanged;
if (alwaysShowValue) {
return this.value;
}

return this.defaultValue || this.value;
}

private getAriaDescribedBy() {
const ids: string[] = [];
if (this.getSupportingText()) {
Expand Down
43 changes: 11 additions & 32 deletions textfield/lib/text-field_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,15 @@ describe('TextField', () => {
describe('resetting the input', () => {
it('should set value back to default value', async () => {
const {harness} = await setupTest();
harness.element.defaultValue = 'Default';
harness.element.setAttribute('value', 'Default');
await env.waitForStability();

expect(harness.element.value).toBe('Default');
await harness.deleteValue();
await harness.inputValue('Value');
expect(harness.element.value).toBe('Value');
harness.element.reset();

expect(harness.element.defaultValue).toBe('Default');
expect(harness.element.value).toBe('Default');
});

Expand All @@ -164,37 +165,15 @@ describe('TextField', () => {
await harness.inputValue('Value');
harness.element.reset();

expect(harness.element.defaultValue).toBe('');
expect(harness.element.value).toBe('');
});

it('should allow defaultValue to update value again', async () => {
const {harness} = await setupTest();

// defaultValue changes value
harness.element.defaultValue = 'First default';
await env.waitForStability();
expect(harness.element.value).toBe('First default');

// Setting value programmatically causes it to stick
harness.element.value = 'Value';
harness.element.defaultValue = 'Second default';
await env.waitForStability();
expect(harness.element.value).toBe('Value');

// Resetting should return to original functionality
harness.element.reset();
harness.element.defaultValue = 'Third default';
await env.waitForStability();
expect(harness.element.value).toBe('Third default');
});
});

describe('default value', () => {
it('should update `value` before user input', async () => {
const {harness} = await setupTest();

harness.element.defaultValue = 'Default';
harness.element.setAttribute('value', 'Default');
await env.waitForStability();

expect(harness.element.value).toBe('Default');
Expand All @@ -203,9 +182,9 @@ describe('TextField', () => {
it('should update `value` multiple times', async () => {
const {harness} = await setupTest();

harness.element.defaultValue = 'First default';
harness.element.setAttribute('value', 'First default');
await env.waitForStability();
harness.element.defaultValue = 'Second default';
harness.element.setAttribute('value', 'Second default');
await env.waitForStability();

expect(harness.element.value).toBe('Second default');
Expand All @@ -214,22 +193,22 @@ describe('TextField', () => {
it('should NOT update `value` after user input', async () => {
const {harness} = await setupTest();

harness.element.defaultValue = 'First default';
harness.element.setAttribute('value', 'First default');
await env.waitForStability();
await harness.deleteValue();
await harness.inputValue('Value');

harness.element.defaultValue = 'Second default';
harness.element.setAttribute('value', 'Second default');
await env.waitForStability();

expect(harness.element.value).toBe('Value');
});

it('should render `value` instead of `defaultValue` when `value` changes',
it('should render `value` instead of default value attribute when `value` changes',
async () => {
const {harness, input} = await setupTest();

harness.element.defaultValue = 'Default';
harness.element.setAttribute('value', 'Default');
await env.waitForStability();
expect(input.value).toBe('Default');

Expand All @@ -240,7 +219,7 @@ describe('TextField', () => {
harness.element.value = '';
await env.waitForStability();
expect(input.value).toBe('');
expect(harness.element.defaultValue).toBe('Default');
expect(harness.element.getAttribute('value')).toBe('Default');
});
});

Expand Down

0 comments on commit 2317c5a

Please sign in to comment.