Skip to content

Commit

Permalink
fix(textfield): counter showing when max length is 0 or removed
Browse files Browse the repository at this point in the history
Fixes #4998

This also fixes an error being thrown in text field's validator when minlength/maxlength change to out of bounds if they're not set in the correct order.

PiperOrigin-RevId: 593860392
  • Loading branch information
asyncLiz authored and copybara-github committed Dec 27, 2023
1 parent c9360e2 commit 5c8fc0f
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 10 deletions.
9 changes: 7 additions & 2 deletions field/internal/field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,16 @@ export class Field extends LitElement {
private readonly slottedAriaDescribedBy!: HTMLElement[];

private get counterText() {
if (this.count < 0 || this.max < 0) {
// Count and max are typed as number, but can be set to null when Lit removes
// their attributes. These getters coerce back to a number for calculations.
const countAsNumber = this.count ?? -1;
const maxAsNumber = this.max ?? -1;
// Counter does not show if count is negative, or max is negative or 0.
if (countAsNumber < 0 || maxAsNumber <= 0) {
return '';
}

return `${this.count} / ${this.max}`;
return `${countAsNumber} / ${maxAsNumber}`;
}

private get supportingOrErrorText() {
Expand Down
16 changes: 12 additions & 4 deletions labs/behaviors/validators/text-field-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,22 @@ export class TextFieldValidator extends Validator<TextFieldState> {
// Use -1 to represent no minlength and maxlength, which is what the
// platform input returns. However, it will throw an error if you try to
// manually set it to -1.
if (state.minLength > -1) {
inputOrTextArea.minLength = state.minLength;
//
// While the type is `number`, it may actually be `null` at runtime.
// `null > -1` is true since `null` coerces to `0`, so we default null and
// undefined to -1.
//
// We set attributes instead of properties since setting a property may
// throw an out of bounds error in relation to the other property.
// Attributes will not throw errors while the state is updating.
if ((state.minLength ?? -1) > -1) {
inputOrTextArea.setAttribute('minlength', String(state.minLength));
} else {
inputOrTextArea.removeAttribute('minlength');
}

if (state.maxLength > -1) {
inputOrTextArea.maxLength = state.maxLength;
if ((state.maxLength ?? -1) > -1) {
inputOrTextArea.setAttribute('maxlength', String(state.maxLength));
} else {
inputOrTextArea.removeAttribute('maxlength');
}
Expand Down
32 changes: 32 additions & 0 deletions labs/behaviors/validators/text-field-validator_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,38 @@ describe('TextFieldValidator', () => {
expect(validity.valueMissing).withContext('valueMissing').toBeFalse();
expect(validationMessage).withContext('validationMessage').toBe('');
});

it('does not throw an error when setting minlength and maxlength out of bounds', () => {
type WritableInputState = {
-readonly [K in keyof InputState]: InputState[K];
};

const state: WritableInputState = {
type: 'text',
value: '',
required: true,
pattern: '',
min: '',
max: '',
minLength: 5,
maxLength: 10,
step: '',
};

const validator = new TextFieldValidator(() => ({
state,
renderedControl: null,
}));

// Compute initial validity with valid minlength of 5 and maxlength of 10
validator.getValidity();
// set to something that is out of bounds of current maxlength="10"
state.minLength = 20;

expect(() => {
validator.getValidity();
}).not.toThrow();
});
});

describe('type="email"', () => {
Expand Down
12 changes: 8 additions & 4 deletions textfield/internal/text-field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,10 @@ export abstract class TextField extends textFieldBaseClass {
// tslint:disable-next-line:no-any
const autocomplete = this.autocomplete as any;

// These properties may be set to null if the attribute is removed, and
// `null > -1` is incorrectly `true`.
const hasMaxLength = (this.maxLength ?? -1) > -1;
const hasMinLength = (this.minLength ?? -1) > -1;
if (this.type === 'textarea') {
return html`
<textarea
Expand All @@ -595,8 +599,8 @@ export abstract class TextField extends textFieldBaseClass {
aria-label=${ariaLabel}
autocomplete=${autocomplete || nothing}
?disabled=${this.disabled}
maxlength=${this.maxLength > -1 ? this.maxLength : nothing}
minlength=${this.minLength > -1 ? this.minLength : nothing}
maxlength=${hasMaxLength ? this.maxLength : nothing}
minlength=${hasMinLength ? this.minLength : nothing}
placeholder=${this.placeholder || nothing}
?readonly=${this.readOnly}
?required=${this.required}
Expand Down Expand Up @@ -631,9 +635,9 @@ export abstract class TextField extends textFieldBaseClass {
?disabled=${this.disabled}
inputmode=${inputMode || nothing}
max=${(this.max || nothing) as unknown as number}
maxlength=${this.maxLength > -1 ? this.maxLength : nothing}
maxlength=${hasMaxLength ? this.maxLength : nothing}
min=${(this.min || nothing) as unknown as number}
minlength=${this.minLength > -1 ? this.minLength : nothing}
minlength=${hasMinLength ? this.minLength : nothing}
pattern=${this.pattern || nothing}
placeholder=${this.placeholder || nothing}
?readonly=${this.readOnly}
Expand Down

0 comments on commit 5c8fc0f

Please sign in to comment.