Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ngOptions is setting select $dirty when options change and selected option is removed #13362

Closed
Narretz opened this issue Nov 22, 2015 · 6 comments

Comments

@Narretz
Copy link
Contributor

Narretz commented Nov 22, 2015

When the options change, and the currently selected option is removed because of that, we update the model and set it to the unknown / empty option. We do this by calling $setViewValue, which calls $setDirty. That means if the options change before the user has interacted with the form, the select is set to $dirty even though there was no user interaction.
We could do the following things:

  1. Remember the $pristine state and restore it if was true
  2. Don't use $setViewValue, as it's not really made for that purpose.

Bug was introduced here: 933591d

@Narretz Narretz added this to the Backlog milestone Nov 22, 2015
@Narretz Narretz changed the title ngOptions is setting $dirty when options change and selected option is removed ngOptions is setting select $dirty when options change and selected option is removed Nov 22, 2015
@petebacondarwin
Copy link
Contributor

OK, so we can do the following:

        // Check to see if the value has changed due to the update to the options
        if (!ngModelCtrl.$isEmpty(previousValue)) {
          var nextValue = selectCtrl.readValue();
          if (ngOptions.trackBy ? !equals(previousValue, nextValue) : previousValue !== nextValue) {
            ngModelCtrl.$viewValue = nextValue;
            ngModelCtrl.$$parseAndValidate();
            ngModelCtrl.$render();
          }
        }

@Narretz do you think this will be acceptable?

@petebacondarwin petebacondarwin modified the milestones: 1.4.8, Backlog Nov 23, 2015
@Narretz
Copy link
Contributor Author

Narretz commented Nov 23, 2015

That looks good, but I think we also need to do ngModelCtrl.$$lastCommittedViewValue = nextValue, because that's what the parse pipeline and the validators use.

@Narretz
Copy link
Contributor Author

Narretz commented Nov 23, 2015

We would still be losing the update to the empty status. Basically, we are just working around the problem. Imo we should be setting the model directly. Or enhance setViewValue so that we can skip the pristine setting in a logical way. Maybe by passing the trigger argument all the way to commitViewValue

@petebacondarwin
Copy link
Contributor

Hmm, setting the model to null eh? That could work.

@Narretz
Copy link
Contributor Author

Narretz commented Nov 24, 2015

I'm not sure if we have the right API for that. We'd need to access the model setter, but ngModel does not expose that. Since no-one else has found the bug, I'm also okay with pushing it back to post-1.4.8

@petebacondarwin petebacondarwin modified the milestones: 1.4.x, 1.4.8 Nov 25, 2015
@Narretz Narretz modified the milestones: 1.4.x, 1.5.x May 27, 2016
@Narretz Narretz modified the milestones: 1.5.x, 1.7.x Apr 12, 2018
@petebacondarwin petebacondarwin modified the milestones: 1.7.x, 1.7.x - won't fix May 16, 2018
@Narretz
Copy link
Contributor Author

Narretz commented May 28, 2018

Not going to get fixed.

@Narretz Narretz closed this as completed May 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants