Material: chips: adding chip on blur no longer working when required is also used

[Splaktar]

Bug, feature request, or proposal:

Bug

What is the expected behavior?

Clicking outside of the chips with input in the chips should create a new chip.
Adding chips and removing chips and adding more chips should work.

What is the current behavior?

We have multi value supported with separator keys of enter, space and tab.
It works good when we have input like (url1 url2) and hit tab to move out of the field
It doesn't transform into chips when we have input like (url1 url2) and click anywhere outside of the field (this used to work before this sync)
After adding chips, if we remove the chips and retry with any input, it doesn't work. Somehow it makes the model as "undefined"

CodePen and steps to reproduce the issue:

CodePen Demo which shows your issue: TBD

Detailed Reproduction Steps: TBD

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

It used to work in 1.1.7.

Which versions of AngularJS, Material, OS, and browsers are affected?

AngularJS Material 1.1.7
Others TBD

Is there anything else we should know? Stack Traces, Screenshots, etc.

Verified that this is fixed by reverting https://github.com/angular/material/pull/11125 in https://github.com/angular/material/pull/11215.

[Splaktar]

@jelbourn can you confirm that they have md-add-on-blur="true" on the md-chips?
Are they using ng-required?

[Splaktar]

Update on the demo:

              <md-chips
                  required
                  ng-model="..."
                  placeholder="..."
                  md-separator-keys="..."
                  md-transform-chip="..."
                  secondary-placeholder="..."
                  md-add-on-blur="...">
              </md-chips>

So assuming that ... is true, then md-add-on-blur="..." is there and is somehow not working.

[IMM0rtalis]

I‘ll try to find a fix/solution the next 48hours

[Splaktar]

@jelbourn It seems like the custom logic in the md-transform-chip expression may be causing this issue. If it starts returning null, this will block chips from being created.

[Splaktar]

I tested on the docs for HEAD and I could not reproduce these issues there. But those docs don't have an example of using md-add-on-blur with required.

So I created a demo that uses md-add-on-blur with required and there does appear to be an issue. Add on blur does not work for the first chip either with the tab key or a mouse click when required is defined. Removing required causes add on blur to work fine again.

[Splaktar]

@hetal0713 we were able to isolate and reproduce this issue. We're working on a test and a fix atm. In the meantime, the code that caused this has been temporarily reverted from master.

[hetal0713]

Ok great. Thank you for the update

[IMM0rtalis]

I am really a bit upset. Finally we have more frequently release cycles but a year ago there were bugs affecting projects and nobody at google cared. Now the project is alive again with +-montly release cycles and a small missbehavior is happening for sb at google and instead of making a real fix or going one version back it is reverted asap without even giving time to fix this since google does not care for features as long as they do not use it. Why can they not just go back to 1.1.7? No, the whole project has to have a revert to make their use case work. -.-

back2topic: how should i make a fix out of master-branch is this code is reverted? Should i make a PR with this feature again + handling this use case? @Splaktar

[Splaktar]

@IMM0rtalis Google doesn't use any version tag of AngularJS Material, so they can't just roll back to an old release. They are based on HEAD. That's why it's critical that we do thorough code reviews and add tests for bugs and new features. This is also why some people's requests to "just merge my PR and clean it up yourself afterwards" cannot be accomodated.

Yes, please make a new PR targeted at master that includes the changes from your previous PR, plus a test that reproduces this failure, and a fix that makes that test pass.

We don't yet have a reproduction for the issue with autocomplete and the ng-change changes, so for now, I would avoid including those changes into your branch (I know that they weren't your original changes).

[Splaktar]

@IMM0rtalis any progress on this? I am going to work on writing a test that reproduces the problem.

[Splaktar]

Verified that both required and ng-required="true" cause this failure.

This test reproduces the issue:

it('should allow adding the first chip on blur when required exists', function() {
  scope.items = [];
  var template =
      '<form name="form">' +
      ' <md-chips name="chips" ng-required="true" ng-model="items" md-add-on-blur="true"></md-chips>' +
      '</form>';

  var element = buildChips(template);
  var ctrl = element.find('md-chips').controller('mdChips');

  element.scope().$apply(function() {
    ctrl.chipBuffer = 'Test';
  });
  element.find('input').triggerHandler('blur');

  expect(scope.form.chips.$error['required']).toBeUndefined();
  expect(scope.items).toEqual(['Test']);
});

[Splaktar]

Opened PR https://github.com/angular/material/pull/11225 to add this functionality back in and fix the regression.

[IMM0rtalis]

Started yesterday before going home; will look into yours when i am at work in 2-3h

[IMM0rtalis]

Yesterday we just found out that we were also affected by this blur-problem :see_no_evil:

Is there a date-guess for releasing 1.1.10 ? - Any feedback of google-teams regarding the problem (is it fixed for them?) @Splaktar

[Splaktar]

This should be in 1.1.9. I am planning to start moving issues and getting prepared for a 1.1.9 release tomorrow. I expect that to take a few days, with a release next week.