fix(md-contact-chips): support md-min-length on md-contact-chips. #9215
Conversation
Add md-min-length to md-contact-chips and propagate attribute to md-autocomplete. Add md-has-not-found=true attribute to md-autocomplete. Fixes #2423.
|
@topherfangio @ThomasBurleson any reason why nothing happened yet (e.g. 'needs review' tags) on this PR? |
| @@ -63,6 +65,8 @@ var MD_CONTACT_CHIPS_TEMPLATE = '\ | |||
| md-items="item in $mdContactChipsCtrl.queryContact($mdContactChipsCtrl.searchText)"\ | |||
| md-item-text="$mdContactChipsCtrl.itemName(item)"\ | |||
| md-no-cache="true"\ | |||
| md-min-length="$mdContactChipsCtrl.minLength"\ | |||
| md-has-not-found="true"\ | |||
There was a problem hiding this comment.
Sorry for the delay; Thomas and I have both been traveling a bit.
Can you describe the necessity of the md-has-not-found here since it is automatically added if we see the associated template?
There was a problem hiding this comment.
@topherfangio when I don't set the md-has-not-found attribute the search function is not called when md-min-length equals 0. I noticed that it was also set to true on the first md-autocomplete demo. If it's better to fix this in another way can you point me in the right direction?
There was a problem hiding this comment.
@topherfangio @devversion while doing further research I found a bug on the master branch that isn't in the v1.1.0-rc5 release. When I remove the md-not-found element in the basic md-autocomplete example I would expect that the search function would still work since the md-min-length equals 0. This is however not the case on anymore on the master branch while seems to work in the v1.1.0-rc5 release.
I can't immediately find which commit broke the functionality.
There was a problem hiding this comment.
@lievenjanssen This is a valid and known bug. We are going to fix this with #9291 (hopefully)
There was a problem hiding this comment.
@devversion @topherfangio I removed the md-has-not-found attribute. This PR is dependent on merge of #9291.
There was a problem hiding this comment.
Still waiting on #9291 to be merged.
@lievenjanssen Can you ping me in a few days if it doesn't get merged so I remember to bring it back up?
There was a problem hiding this comment.
@topherfangio a ping as requested ;-) #9291 is merged in the meantime.
topherfangio
added
the
needs: feedback
devversion
added
the
Blocked
|
@devversion will this PR be merged together with #9291 and is the milestone still 1.1.1? |
|
@lievenjanssen I hope so - still waiting for @topherfangio's manual testing of #9291. |
topherfangio
removed
the
needs: feedback
ThomasBurleson
added
needs: presubmit
and removed
Blocked
ThomasBurleson
added
needs: unit tests
|
@lievenjanssen please provide 1 or more unit tests. |
ThomasBurleson
added
the
in progress
|
@ThomasBurleson I added a test to check if minLength on autocomplete controller equals md-min-length attribute on md-contact-chips element. |
| @@ -82,6 +83,15 @@ describe('<md-contact-chips>', function() { | |||
| expect(chip.find('img').length).toBe(0); | |||
| }); | |||
|
|
|||
| it('check if minLength on autocomplete controller equals md-min-length attribute on md-contact-chips element', inject(function() { | |||
There was a problem hiding this comment.
Can we rename this to should forward md-min-length attribute to the autocomplete ?
devversion
added
needs: review
topherfangio
added
needs: presubmit
and removed
needs: review
|
Am I correct that the presubmit prerequisite (#9291) is fulfilled and that this can be merged? |
Add md-min-length to md-contact-chips and propagate attribute to md-autocomplete.
Add md-has-not-found=true attribute to md-autocomplete.
Fixes #2423.