Skip to content

Commit

Permalink
fix(material-experimental/mdc-form-field): show required asterisk for…
Browse files Browse the repository at this point in the history
… disabled fields

We currently implement our own logic for the asterisk. We need to update
our conditions for showing the asterisk to match with MDC. i.e. in MDC
the required asterisk shows always, regardless of the disabled state.

This is different to the standard form-field, but we want to match this
with the MDC text-field.

Additionally, this commit cleans up our custom asterisk code as we can
now fully rely on MDC's floating label implementation for the asterisk
styles. The floating label wasn't flexible enough in the past.

Finally, this also simplifies the floating label directive for more
optimized payload size. We don't need the getters/setters and
MDC foundation/component proxy for basically toggling simple
public API classes (or measuring the label width). Foundations
and components generally contribute a lot to payload size
due to their natural concept of providing default and fallback
adapters. We'll be working on outlining these issues long-term,
but generally there should be no reason in complicating this
by using one of the these MDC components/foundations.

Related to #19410
  • Loading branch information
devversion authored and andrewseguin committed Jun 12, 2020
1 parent c7cadc3 commit e3413ba
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 71 deletions.
4 changes: 4 additions & 0 deletions src/dev-app/mdc-input/mdc-input-demo.html
Expand Up @@ -264,6 +264,10 @@ <h4>Textarea</h4>
<mat-label>Disabled field</mat-label>
<input matInput disabled value="Value">
</mat-form-field>
<mat-form-field>
<mat-label>Label</mat-label>
<input matInput disabled required>
</mat-form-field>
<mat-form-field>
<mat-label>Required field</mat-label>
<input matInput required>
Expand Down
1 change: 0 additions & 1 deletion src/material-experimental/mdc-form-field/BUILD.bazel
Expand Up @@ -24,7 +24,6 @@ ng_module(
"//src/material/core",
"//src/material/form-field",
"@npm//@angular/forms",
"@npm//@material/floating-label",
"@npm//@material/line-ripple",
"@npm//@material/textfield",
"@npm//rxjs",
Expand Down
Expand Up @@ -6,16 +6,6 @@
// styles to fit our needs. See individual comments for context on why
// certain MDC styles need to be modified.
@mixin _mat-mdc-text-field-structure-overrides() {
// Always hide the asterisk displayed by MDC. This is necessary because MDC can only display
// the asterisk if the label is directly preceded by the input. MDC does this because it
// relies on CSS to figure out if the input/textarea is required. This does not apply for us
// because it's not guaranteed that the form control is an input/textarea. The required state
// is handled as part of the registered form-field control instance. The asterisk will be
// rendered conditionally through the floating label.
.mat-mdc-form-field .mdc-floating-label::after {
display: none;
}

// Unset the border set by MDC. We move the border (which serves as the Material Design
// text-field bottom line) into its own element. This is necessary because we want the
// bottom-line to span across the whole form-field (including prefixes and suffixes). Also
Expand Down
Expand Up @@ -6,40 +6,41 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, ElementRef, Input, OnDestroy} from '@angular/core';
import {MDCFloatingLabel} from '@material/floating-label';
import {Directive, ElementRef, Input} from '@angular/core';
import {ponyfill} from '@material/dom';

/**
* Internal directive that creates an instance of the MDC floating label
* component. Using a directive allows us to conditionally render a floating label
* in the template without having to manually instantiate the `MDCFloatingLabel` component.
* Internal directive that maintains a MDC floating label. This directive does not
* use the `MDCFloatingLabelFoundation` class, as it is not worth the size cost of
* including it just to measure the label width and toggle some classes.
*
* The component is responsible for setting up the floating label styles, and for providing
* an @Input that can be used by the form-field to toggle floating state of the label.
* The use of a directive allows us to conditionally render a floating label in the
* template without having to manually manage instantiation and destruction of the
* floating label component based on.
*
* The component is responsible for setting up the floating label styles, measuring label
* width for the outline notch, and providing inputs that can be used to toggle the
* label's floating or required state.
*/
@Directive({
selector: 'label[matFormFieldFloatingLabel]',
host: {
'class': 'mdc-floating-label',
'[class.mdc-floating-label--required]': 'required',
'[class.mdc-floating-label--float-above]': 'floating',
},
})
export class MatFormFieldFloatingLabel extends MDCFloatingLabel implements OnDestroy {
@Input()
get floating() { return this._floating; }
set floating(shouldFloat: boolean) {
if (shouldFloat !== this._floating) {
this._floating = shouldFloat;
this.float(shouldFloat);
}
}
private _floating = false;
export class MatFormFieldFloatingLabel {
/** Whether the label is floating. */
@Input() floating: boolean = false;
/** Whether the label is required. */
@Input() required: boolean = false;

constructor(private _elementRef: ElementRef) {
super(_elementRef.nativeElement);
}
constructor(private _elementRef: ElementRef) {}

ngOnDestroy() {
this.destroy();
/** Gets the width of the label. Used for the outline notch. */
getWidth(): number {
return ponyfill.estimateScrollWidth(this._elementRef.nativeElement);
}

/** Gets the HTML element for the floating label. */
Expand Down
11 changes: 3 additions & 8 deletions src/material-experimental/mdc-form-field/form-field.html
Expand Up @@ -14,21 +14,16 @@
*Note*: We add aria-owns as a workaround for an issue in JAWS & NVDA where the label isn't
read if it comes before the control in the DOM.
-->
<label matFormFieldFloatingLabel [floating]="_shouldLabelFloat()"
<label matFormFieldFloatingLabel
[floating]="_shouldLabelFloat()"
[required]="!hideRequiredMarker && _control.required"
*ngIf="_hasFloatingLabel()"
(cdkObserveContent)="_refreshOutlineNotchWidth()"
[cdkObserveContentDisabled]="!_hasOutline()"
[id]="_labelId"
[attr.for]="_control.id"
[attr.aria-owns]="_control.id">
<ng-content select="mat-label"></ng-content>

<!-- Manually handle the required asterisk. This is necessary because MDC can only
display the asterisk if the label is directly preceded by the input. This cannot
be guaranteed here since the form control is not necessarily an input, or is wrapped.
-->
<span class="mat-mdc-form-field-required-marker" aria-hidden="true"
*ngIf="!hideRequiredMarker && _control.required && !_control.disabled">&#32;*</span>
</label>
</ng-template>

Expand Down
7 changes: 4 additions & 3 deletions src/material-experimental/mdc-form-field/form-field.ts
Expand Up @@ -244,9 +244,10 @@ export class MatFormField implements AfterViewInit, OnDestroy, AfterContentCheck
// want to update the notch whenever the `_shouldLabelFloat()` value changes.
getLabelWidth: () => 0,

// TODO: MDC now supports setting the required asterisk marker directly on
// the label component. This adapter method may be implemented and
// mat-mdc-form-field-required-marker removed.
// We don't use `setLabelRequired` as it relies on a mutation observer for determining
// when the `required` state changes. This is not reliable and flexible enough for
// our form field, as we support custom controls and detect the required state through
// a public property in the abstract form control.
setLabelRequired: () => {},
notchOutline: () => {},
closeOutline: () => {},
Expand Down
Expand Up @@ -50,7 +50,7 @@ ng_web_test_suite(
"@npm//:node_modules/@material/textfield/dist/mdc.textfield.js",
"@npm//:node_modules/@material/line-ripple/dist/mdc.lineRipple.js",
"@npm//:node_modules/@material/notched-outline/dist/mdc.notchedOutline.js",
"@npm//:node_modules/@material/floating-label/dist/mdc.floatingLabel.js",
"@npm//:node_modules/@material/dom/dist/mdc.dom.js",
],
deps = [
":unit_tests_lib",
Expand Down
2 changes: 1 addition & 1 deletion src/material-experimental/mdc-input/BUILD.bazel
Expand Up @@ -60,10 +60,10 @@ ng_test_library(
ng_web_test_suite(
name = "unit_tests",
static_files = [
"@npm//:node_modules/@material/dom/dist/mdc.dom.js",
"@npm//:node_modules/@material/textfield/dist/mdc.textfield.js",
"@npm//:node_modules/@material/line-ripple/dist/mdc.lineRipple.js",
"@npm//:node_modules/@material/notched-outline/dist/mdc.notchedOutline.js",
"@npm//:node_modules/@material/floating-label/dist/mdc.floatingLabel.js",
],
deps = [
":input_tests_lib",
Expand Down
41 changes: 17 additions & 24 deletions src/material-experimental/mdc-input/input.spec.ts
Expand Up @@ -315,48 +315,41 @@ describe('MatMdcInput without forms', () => {
}));

it('supports label required star', fakeAsync(() => {
let fixture = createComponent(MatInputLabelRequiredTestComponent);
const fixture = createComponent(MatInputLabelRequiredTestComponent);
fixture.detectChanges();

let el = fixture.debugElement.query(By.css('label'))!;
expect(el).not.toBeNull();
expect(el.nativeElement.textContent).toBe('hello *');
const label = fixture.debugElement.query(By.css('label'))!;
expect(label).not.toBeNull();
expect(label.nativeElement.textContent).toBe('hello');
expect(label.nativeElement.classList).toContain('mdc-floating-label--required');
}));

it('should hide the required star if input is disabled', () => {
it('should not hide the required star if input is disabled', () => {
const fixture = createComponent(MatInputLabelRequiredTestComponent);

fixture.componentInstance.disabled = true;
fixture.detectChanges();

const el = fixture.debugElement.query(By.css('label'))!;

expect(el).not.toBeNull();
expect(el.nativeElement.textContent).toBe('hello');
const label = fixture.debugElement.query(By.css('label'))!;
expect(label).not.toBeNull();
expect(label.nativeElement.textContent).toBe('hello');
expect(label.nativeElement.classList).toContain('mdc-floating-label--required');
});

it('should hide the required star from screen readers', fakeAsync(() => {
let fixture = createComponent(MatInputLabelRequiredTestComponent);
fixture.detectChanges();

let el = fixture.debugElement
.query(By.css('.mat-mdc-form-field-required-marker'))!.nativeElement;

expect(el.getAttribute('aria-hidden')).toBe('true');
}));

it('hide label required star when set to hide the required marker', fakeAsync(() => {
let fixture = createComponent(MatInputLabelRequiredTestComponent);
const fixture = createComponent(MatInputLabelRequiredTestComponent);
fixture.detectChanges();

let el = fixture.debugElement.query(By.css('label'))!;
expect(el).not.toBeNull();
expect(el.nativeElement.textContent).toBe('hello *');
const label = fixture.debugElement.query(By.css('label'))!;
expect(label).not.toBeNull();
expect(label.nativeElement.classList).toContain('mdc-floating-label--required');
expect(label.nativeElement.textContent).toBe('hello');

fixture.componentInstance.hideRequiredMarker = true;
fixture.detectChanges();

expect(el.nativeElement.textContent).toBe('hello');
expect(label.nativeElement.classList).not.toContain('mdc-floating-label--required');
expect(label.nativeElement.textContent).toBe('hello');
}));

it('supports the disabled attribute as binding', fakeAsync(() => {
Expand Down
2 changes: 1 addition & 1 deletion src/material-experimental/mdc-input/testing/BUILD.bazel
Expand Up @@ -39,7 +39,7 @@ ng_web_test_suite(
"@npm//:node_modules/@material/textfield/dist/mdc.textfield.js",
"@npm//:node_modules/@material/line-ripple/dist/mdc.lineRipple.js",
"@npm//:node_modules/@material/notched-outline/dist/mdc.notchedOutline.js",
"@npm//:node_modules/@material/floating-label/dist/mdc.floatingLabel.js",
"@npm//:node_modules/@material/dom/dist/mdc.dom.js",
],
deps = [
":unit_tests_lib",
Expand Down

0 comments on commit e3413ba

Please sign in to comment.