Repository navigation
fix(input): ng-required conflicts with ng-true-value - #6065
tameraydin wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
You should also test that the expected behaviour occurs when the field is empty. I also want to see lots of
expect(inputElm).toBeValid(); // or toBeInvalid(); if it makes sensein this spec.
There was a problem hiding this comment.
Ok, now the tests also cover validation status.
ghost
assigned
|
@matsko this closes an issue assigned to you, do you want to merge this? IMO it LGTM, although the change to $isEmpty doesn't really make sense to me, I don't see why it works to compare to "true". So that might deserve a closer look. |
|
@caitp I'm not sure who assigned it to me in the first place. If you want to take it over then that works, otherwise I can review it. |
|
It's already got an LGTM from me based on the tests, I just haven't looked into why it actually works, it might be dependent on what the formatter stack looks like. I expect it's probably good, though. |
|
Hi @caitp, BTW, I see the build is not completed due to a missing package error, -but the code was successfully passed-, do I have anything to do with it? |
|
I don't think you should worry about that, but you should rebase your code as the travis tests have changed, and it will make life easier when we merge this. |
Before this change, overridden "isEmpty" method which is called in "requiredDirective" would return a wrong result when the parameter is given as "true". This change corrects the statement value within method. Closes #5164
02dc2aa to
fd2d6c0
Compare
cad9560 to
f294244
Compare
e8dc429 to
e83fab9
Compare
|
This was fixed and works in current rc / snapshot. (1.3.0-rc.5) |
Before this change, overridden "isEmpty" method which is called in "requiredDirective" would return a wrong result when the parameter is given as "true". This change corrects the statement value within method.
Closes #5164