Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ErrorStateMatcher for checkbox and slide-toggle #6591

Closed
gise88 opened this issue Aug 22, 2017 · 8 comments
Closed

ErrorStateMatcher for checkbox and slide-toggle #6591

gise88 opened this issue Aug 22, 2017 · 8 comments
Assignees
Labels
area: material/checkbox area: material/slide-toggle P5 The team acknowledges the request but does not plan to address it, it remains open for discussion

Comments

@gise88
Copy link

gise88 commented Aug 22, 2017

Bug, feature request, or proposal:

Question/Bug

What is the expected behavior?

I don't know if the current is the right behavior but I see some discrepancies between the behaviors of the input and the slide-toggle.

Slide-toggle gets its errors even when is pristine or not touched. The input gets the errors only when is not pristine/touched or when you try to submit the form.

What is the current behavior & what are the steps to reproduce?

go to: https://plnkr.co/edit/uofeSTyA2xAHEHR8RMq0?p=preview
and simply look at the difference.
The slide-toggle already has its errors.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

angular version: 4.3.5
material version: 2.0.0-beta.8-6cdbf36

Is there anything else we should know?

Am I missing something? Is this the right procedure? Don't you think it is better to create a container like md-input-container for the checkbox and slide-toggle?

@devversion
Copy link
Member

The behavior seems to be different due to the ErrorStateMatcher approach for inputs.

https://github.com/angular/material2/blob/595cffdcff7b393a9a01fbca2b84b78573bbbaad/src/lib/input/input.ts#L223

@devversion
Copy link
Member

Under the hood both form controls are having the required error set.

For the input it just doesn't show up because the errorStateMatcher is hiding the <md-error> elements inside of the input container.

https://github.com/angular/material2/blob/4d82f833c7e4a22814b8e225f05ef4ba01a86ad6/src/lib/form-field/form-field.html#L42-L46

cc. @mmalerba @willshowell

@willshowell
Copy link
Contributor

The errorStateMatcher logic belongs specifically to the input (and soon the select #6147). I dunno the future of the MdFormField, but it seems like if there's a way to capture error logic there, that would be great!

<md-form-field>
  <md-slide-toggle formControlName="termsToggle">
    I agree to terms and conditions
  </md-slide-toggle>
  <md-error>You must agree</md-error>
</md-form-field>

As I recall from point 1 of #4750 (comment), it was #4757 that "Moves the _isErrorState logic to the input child to avoid a circular dependency".

cc @crisbeto too

@devversion
Copy link
Member

@willshowell Yeah I agree that this would be a nice way to handle errors here.

In regards to the form field, I'm not sure how well it would play together since the md-form-field element normally supports bindings like floatPlaceholder (would probably cause an ambiguous API)

@josephperrott josephperrott added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Sep 27, 2017
@mmalerba
Copy link
Contributor

mmalerba commented Nov 21, 2017

I think the mat-slide-toggle (and all material form controls) should use the ErrorStateMatcher, but I don't think its a good fit to go in mat-form-field. I as actually thinking about creating a directive to set the ErrorStateMatcher that targets the [matErorrStateMatcher] selector. That way we could apply it to any element to switch out the provider for all of its children. Then we would't have to add separate inputs to every component and we could also add it on <div>s and things

@devversion devversion changed the title [Question/Bug?] Errors added to slide-toggle even when it is still pristine or untouched ErrorStateMatcher for checkbox and slide-toggle Nov 27, 2017
@xanscale
Copy link

xanscale commented Mar 16, 2018

inside validators.d.ts there are 2 Required Validator, one for "any controls" and one for "checkbox controls"

i think mat-slide-toggle should use CheckboxRequiredValidator instead of RequiredValidator

/**
 * A Directive that adds the `required` validator to any controls marked with the
 * `required` attribute, via the {@link NG_VALIDATORS} binding.
 *
 * ### Example
 *
 * ```
 * <input name="fullName" ngModel required>
 * ```
 *
 * @stable
 */
export declare class RequiredValidator implements Validator {
    private _required;
    private _onChange;
    required: boolean | string;
    validate(c: AbstractControl): ValidationErrors | null;
    registerOnValidatorChange(fn: () => void): void;
}
/**
 * A Directive that adds the `required` validator to checkbox controls marked with the
 * `required` attribute, via the {@link NG_VALIDATORS} binding.
 *
 * ### Example
 *
 * ```
 * <input type="checkbox" name="active" ngModel required>
 * ```
 *
 * @experimental
 */
export declare class CheckboxRequiredValidator extends RequiredValidator {
    validate(c: AbstractControl): ValidationErrors | null;
}

@devversion devversion added area: material/checkbox area: material/slide-toggle P5 The team acknowledges the request but does not plan to address it, it remains open for discussion and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels May 27, 2020
@crisbeto
Copy link
Member

It looks like the slide toggle now uses the CheckboxRequiredValidator for its custom required validator which should resolve this issue.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/checkbox area: material/slide-toggle P5 The team acknowledges the request but does not plan to address it, it remains open for discussion
Projects
None yet
Development

No branches or pull requests

7 participants