angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.35k stars 6.74k forks source link

[Docs] More details on custom form field controls guide #13624

Open michael-letcher opened 6 years ago

michael-letcher commented 6 years ago

Bug, feature request, or proposal:

I'd like to request a review of the 'Creating a custom form field control' guide as it does not cover enough in regards to each of it properties and opts to just mentioning them.

What is the expected behaviour?

Common examples that are detailed on other approaches and usages.

What is the current behaviour?

Common examples that are not all detailed on other approaches or usages.

What is the use-case or motivation for changing an existing behaviour?

My biggest gripe that pushed me to make this ticket is that of the error state.

Since we're not using an NgControl in this example, we don't need to do anything other than just set it to false.

Where as it should be like 'ngControl' which has sufficient methods of implementation

It is likely you will want to implement ControlValueAccessor ...

Is there anything else we should know?

Others have had the same issue

https://stackoverflow.com/questions/52017000/angular-material-custom-reactive-form-control-with-error-state/52017769

michahell commented 6 years ago

add this to the (list of pains devs go through because they don't understand the) documentation:

I think what is needed most, is how custom form controls work for template driven forms and for reactive forms, and which directives and interfaces should- and should not be added / implemented. We recently just implemented our own working custom reactive form control but it took us a week of figuring out how exactly it should work... :/

mmalerba commented 5 years ago

Adding help wanted label, I think this would be a great place for someone who has gone through the process and knows where the documentation gaps are to contribute and improve

guilhermetod commented 3 years ago

@mmalerba I'm looking into some way to help on a simpler example for this. I'm considering something that could be a little more reusable, maybe a simple auto-formatting input, just so that the user understand how the implementation works. I think the current example is a little bit too complex for begginers.

In the meanwhile, I found a doc bug on the current guide with the ElementRef. The "focused" and "onContainerClick" sections refer to the ElementRef as elRef, while the setDescribedByIds section refer to it as _elementRef.

focused

This property indicates whether the form field control should be considered to be in a focused state. When it is in a focused state, the form field is displayed with a solid color underline. For the purposes of our component, we want to consider it focused if any of the part inputs are focused. We can use the FocusMonitor from @angular/cdk to easily check this. We also need to remember to emit on the stateChanges stream so change detection can happen.

focused = false;

constructor(fb: FormBuilder, private fm: FocusMonitor, private elRef: ElementRef<HTMLElement>) {
  ...
  fm.monitor(elRef, true).subscribe(origin => {
    this.focused = !!origin;
    this.stateChanges.next();
  });
}

ngOnDestroy() {
  ...
  this.fm.stopMonitoring(this.elRef);
}

setDescribedByIds(ids: string[])

This method is used by the <mat-form-field> to set element ids that should be used for the aria-describedby attribute of your control. The ids are controlled through the form field as hints or errors are conditionally displayed and should be reflected in the control's aria-describedby attribute for an improved accessibility experience.

The setDescribedByIds method is invoked whenever the control's state changes. Custom controls need to implement this method and update the aria-describedby attribute based on the specified element ids. Below is an example that shows how this can be achieved.

Note that the method by default will not respect element ids that have been set manually on the control element through the aria-describedby attribute. To ensure that your control does not accidentally override existing element ids specified by consumers of your control, create an input called userAriaDescribedby like followed:

@Input('aria-describedby') userAriaDescribedBy: string;

The form field will then pick up the user specified aria-describedby ids and merge them with ids for hints or errors whenever setDescribedByIds is invoked.

setDescribedByIds(ids: string[]) {
  const controlElement = this._elementRef.nativeElement
    .querySelector('.example-tel-input-container')!;
  controlElement.setAttribute('aria-describedby', ids.join(' '));
}

I can submit a PR to fix this, I'm just wondering which reference should I use. The _elementRef matches the form-field example. I can change this and match the rest of the guide to output the form field example, which may make the guide a little less confusing for now. What do you think?

mmalerba commented 3 years ago

Hi @guilhermetod, thanks for helping to fix up the docs! I think it would be best to use elRef or elementRef in the example code. The underscore for private methods is a style convention we use in our code, but for examples it probably just makes them harder to read.

guilhermetod commented 3 years ago

Nice @mmalerba

So, may I remove the underscore from the examples too, so that the guide matches the example in the end.

mmalerba commented 3 years ago

Sure, that sounds good. If it turns out that our linter is overzealous and complains about the naming we might just have to leave the underscore in, but lets try it with no underscore first.