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

MatAutocomplete directive with undefined value should be ignored and not throw exception #11096

Closed
gtranter opened this issue May 1, 2018 · 5 comments · Fixed by #11142
Closed
Assignees
Labels
feature This issue represents a new feature or feature request rather than a bug or bug fix needs: discussion Further discussion with the team is needed before proceeding P4 A relatively minor issue that is not relevant to core functions

Comments

@gtranter
Copy link

gtranter commented May 1, 2018

Bug, feature request, or proposal:

Feature request

What is the expected behavior?

When the value for a MatInput's [matAutocomplete] option is null/undefined, the MatAutocompleteTrigger should not do anything.

What is the current behavior?

If a MatInput with an undefined [matAutocomplete] option is clicked, an exception is thrown (deliberately) by the MatAutocompleteTrigger.

What are the steps to reproduce?

https://stackblitz.com/edit/angular-mvsaab?embed=1&file=component/my-autocomplete.html
Click on the "No autocomplete" field and view the console.

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

In order to create a custom component that wraps a MatFormField with MatInput and can optionally be used as a MatAutocomplete, the bound directive value needs to be allowed to be null/undefined. I understand the purpose of the exception and the intent of the current design, but there should be some way to disable the trigger other than not using the directive so that it can be wrapped as described and shown in the stackblitz example.

This ties in to another limitation - using *ngIf on the MatInput inside MatFormField. If that worked (it doesn't), then it would serve as a workaround for this problem as I could easily have separate MatInput templates for with or without a [matAutocomplete] option.

The only way I have found to work around this is to actually *ngIf the entire MatFormField in my component's template. Not only is this far from ideal, but it leads to another problem - when I use ng-content selectors inside the form field (I do this for content inside the element), I cannot use the same selector in both form field definitions (it seems that the first in the template is used which may be in the form field excluded by the *ngIf).

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

latest 5.x

Is there anything else we should know?

If there is a reasonable workaround for any of these problems, that would be fabulously useful.

EDIT: I've managed to work around the selector issue by moving the ng-content into an ng-template and pointing to the template from within the mat-error elements.

@crisbeto
Copy link
Member

crisbeto commented May 2, 2018

I can see your reasoning @gtranter, but I feel that changing the current behavior might lead to more confusion (e.g. making it so it only throws for something like undefined, but then in all other cases it doesn't do anything).

@gtranter
Copy link
Author

gtranter commented May 2, 2018

I'm not married to changing the existing behavior actually - I am just hoping for a way around it - perhaps in the way of a property for the directive to simply turn off the trigger. For example:

<mat-form-field>
    <input type="text" [matAutocomplete]="auto" [matAutocompleteDisabled]="!auto">
</mat-form-field>

@crisbeto
Copy link
Member

crisbeto commented May 3, 2018

I'm not convinced that this won't introduce confusion between the native disabled attribute and whatever disables the autocomplete.

@gtranter
Copy link
Author

gtranter commented May 3, 2018

Then don't use that exact name - it was just an example - come up with something more appropriate and less confusing.

@crisbeto crisbeto added feature This issue represents a new feature or feature request rather than a bug or bug fix has pr P4 A relatively minor issue that is not relevant to core functions labels May 3, 2018
crisbeto added a commit to crisbeto/material2 that referenced this issue May 3, 2018
Allows for the autocomplete trigger to be disabled without disabling the input or making it readonly. When disabled, all autocomplete-related accessibility will be removed and the user won't be able to open the panel. This is useful for cases where the input might be optionally become an autocomplete, based on certain conditions, which usually isn't possible because directives can't be applied conditionally.

Fixes angular#11096.
josephperrott pushed a commit that referenced this issue May 4, 2018
Allows for the autocomplete trigger to be disabled without disabling the input or making it readonly. When disabled, all autocomplete-related accessibility will be removed and the user won't be able to open the panel. This is useful for cases where the input might be optionally become an autocomplete, based on certain conditions, which usually isn't possible because directives can't be applied conditionally.

Fixes #11096.
@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 8, 2019
@mmalerba mmalerba added the needs: discussion Further discussion with the team is needed before proceeding label Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature This issue represents a new feature or feature request rather than a bug or bug fix needs: discussion Further discussion with the team is needed before proceeding P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants