Skip to content

Commit

Permalink
fix(material/input): incorrect color for select using the size attrib…
Browse files Browse the repository at this point in the history
…ute (#23734)

Currently we don't apply the theme colors to native `select` elements, because it can the option text to blend in with the background for Windows users. Our current approach seems to break down for inline selects (with `multiple` or `size` attributes), because they don't have a dropdown, but are instead rendered inline.

These changes make it so the exclusion is only applied to `select` elements that have a dropdown.

(cherry picked from commit 032cb06)
  • Loading branch information
crisbeto authored and andrewseguin committed Oct 19, 2021
1 parent 38e26e9 commit 0500652
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 7 deletions.
Expand Up @@ -80,7 +80,7 @@ $mat-form-field-select-horizontal-end-padding: $mat-form-field-select-arrow-widt
$dropdown-icon-color: rgba(mdc-theme-color.prop-value(on-surface), 0.54);
$disabled-dropdown-icon-color: rgba(mdc-theme-color.prop-value(on-surface), 0.38);

select.mat-mdc-form-field-input-control {
select.mat-mdc-form-field-input-control:not(.mat-mdc-native-select-inline) {
// On dark themes we set the native `select` color to some shade of white,
// however the color propagates to all of the `option` elements, which are
// always on a white background inside the dropdown, causing them to blend in.
Expand Down
31 changes: 31 additions & 0 deletions src/material-experimental/mdc-input/input.spec.ts
Expand Up @@ -659,6 +659,37 @@ describe('MatMdcInput without forms', () => {
expect(labelEl.classList).toContain('mdc-floating-label--float-above');
}));

it('should mark a multi-select as being inline', fakeAsync(() => {
const fixture = createComponent(MatInputSelect);
fixture.detectChanges();

const select: HTMLSelectElement = fixture.nativeElement.querySelector('select');

expect(select.classList).not.toContain('mat-mdc-native-select-inline');

select.multiple = true;
fixture.detectChanges();

expect(select.classList).toContain('mat-mdc-native-select-inline');
}));

it('should mark a select with a size as being inline', fakeAsync(() => {
const fixture = createComponent(MatInputSelect);
fixture.detectChanges();

const select: HTMLSelectElement = fixture.nativeElement.querySelector('select');

expect(select.classList).not.toContain('mat-mdc-native-select-inline');

select.size = 3;
fixture.detectChanges();
expect(select.classList).toContain('mat-mdc-native-select-inline');

select.size = 1;
fixture.detectChanges();
expect(select.classList).not.toContain('mat-mdc-native-select-inline');
}));

it('should not float the label if the selectedIndex is negative', fakeAsync(() => {
const fixture = createComponent(MatInputSelect);
fixture.detectChanges();
Expand Down
2 changes: 2 additions & 0 deletions src/material-experimental/mdc-input/input.ts
Expand Up @@ -26,10 +26,12 @@ import {MatInput as BaseMatInput} from '@angular/material/input';
'[class.mat-form-field-autofill-control]': 'false',
'[class.mat-input-element]': 'false',
'[class.mat-form-field-control]': 'false',
'[class.mat-native-select-inline]': 'false',
'[class.mat-input-server]': '_isServer',
'[class.mat-mdc-form-field-textarea-control]': '_isInFormField && _isTextarea',
'[class.mat-mdc-form-field-input-control]': '_isInFormField',
'[class.mdc-text-field__input]': '_isInFormField',
'[class.mat-mdc-native-select-inline]': '_isInlineSelect()',
// Native input properties that are overwritten by Angular inputs need to be synced with
// the native input element. Otherwise property bindings for those don't work.
'[id]': 'id',
Expand Down
14 changes: 8 additions & 6 deletions src/material/input/_input-theme.scss
Expand Up @@ -37,12 +37,14 @@
// Since we can't change background of the dropdown, we need to explicitly
// reset the color of the options to something dark.
@if (map.get($config, is-dark)) {
option {
color: palette.$dark-primary-text;
}

option:disabled {
color: palette.$dark-disabled-text;
&:not(.mat-native-select-inline) {
option {
color: palette.$dark-primary-text;
}

option:disabled {
color: palette.$dark-disabled-text;
}
}
}
}
Expand Down
31 changes: 31 additions & 0 deletions src/material/input/input.spec.ts
Expand Up @@ -732,6 +732,37 @@ describe('MatInput without forms', () => {
expect(formFieldEl.classList).toContain('mat-form-field-should-float');
}));

it('should mark a multi-select as being inline', fakeAsync(() => {
const fixture = createComponent(MatInputSelect);
fixture.detectChanges();

const select: HTMLSelectElement = fixture.nativeElement.querySelector('select');

expect(select.classList).not.toContain('mat-native-select-inline');

select.multiple = true;
fixture.detectChanges();

expect(select.classList).toContain('mat-native-select-inline');
}));

it('should mark a select with a size as being inline', fakeAsync(() => {
const fixture = createComponent(MatInputSelect);
fixture.detectChanges();

const select: HTMLSelectElement = fixture.nativeElement.querySelector('select');

expect(select.classList).not.toContain('mat-native-select-inline');

select.size = 3;
fixture.detectChanges();
expect(select.classList).toContain('mat-native-select-inline');

select.size = 1;
fixture.detectChanges();
expect(select.classList).not.toContain('mat-native-select-inline');
}));

it('should not float the label if the selectedIndex is negative', fakeAsync(() => {
const fixture = createComponent(MatInputSelect);
fixture.detectChanges();
Expand Down
7 changes: 7 additions & 0 deletions src/material/input/input.ts
Expand Up @@ -80,6 +80,7 @@ const _MatInputBase = mixinErrorState(
'[disabled]': 'disabled',
'[required]': 'required',
'[attr.readonly]': 'readonly && !_isNativeSelect || null',
'[class.mat-native-select-inline]': '_isInlineSelect()',
// Only mark the input as invalid for assistive technology if it has a value since the
// state usually overlaps with `aria-required` when the input is empty and can be redundant.
'[attr.aria-invalid]': '(empty && required) ? null : errorState',
Expand Down Expand Up @@ -507,6 +508,12 @@ export class MatInput
}
}

/** Whether the form control is a native select that is displayed inline. */
_isInlineSelect(): boolean {
const element = this._elementRef.nativeElement as HTMLSelectElement;
return this._isNativeSelect && (element.multiple || element.size > 1);
}

static ngAcceptInputType_disabled: BooleanInput;
static ngAcceptInputType_readonly: BooleanInput;
static ngAcceptInputType_required: BooleanInput;
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/material/input.md
Expand Up @@ -60,6 +60,7 @@ export class MatInput extends _MatInputBase implements MatFormFieldControl<any>,
protected _id: string;
protected _isBadInput(): boolean;
readonly _isInFormField: boolean;
_isInlineSelect(): boolean;
readonly _isNativeSelect: boolean;
protected _isNeverEmpty(): boolean;
readonly _isServer: boolean;
Expand Down

0 comments on commit 0500652

Please sign in to comment.