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

fix(input): keep track of min/max attrs on-the-fly #4553

Closed
wants to merge 1 commit into from

Conversation

runk
Copy link
Contributor

@runk runk commented Oct 21, 2013

Now input[type=button] keeping track of both min and max attrs even if they change over time.

Now input[type=button] keeping track of both min and max attrs ever if they change over time.
@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@runk
Copy link
Contributor Author

runk commented Oct 21, 2013

CLA / Real name is Dmitry Shirokov deadrunk@gmail.com

@ghost ghost assigned matsko Oct 21, 2013
@IgorMinar
Copy link
Contributor

LGTM

@matsko this is assigned to you. are you waiting for something before merging it? I'm not going to merge it now in case you are waiting for something...

@matsko
Copy link
Contributor

matsko commented Oct 24, 2013

@IgorMinar Yup. Merging it now. At the time I couldn't verify the CLA thing.

@matsko
Copy link
Contributor

matsko commented Oct 24, 2013

MERGED

@matsko
Copy link
Contributor

matsko commented Oct 24, 2013

Landed as 4b653ae

@matsko matsko closed this Oct 24, 2013
@quazzie
Copy link
Contributor

quazzie commented Nov 5, 2013

This does not re-validate the value when changing min/max.
Does not test form-validity.

If i change min/max the control is still valid even if the value is out of range.

@quazzie
Copy link
Contributor

quazzie commented Nov 5, 2013

And input[type=button] ?

@runk
Copy link
Contributor Author

runk commented Nov 5, 2013

@quazzie could you show your code please? On the jsfiddle or somewhere else.

@quazzie
Copy link
Contributor

quazzie commented Nov 5, 2013

@quazzie
Copy link
Contributor

quazzie commented Nov 5, 2013

#4788

@runk
Copy link
Contributor Author

runk commented Nov 5, 2013

@quazzie I figured out why this worked for me - I changed the model's value at the same time with the min's value. It triggers the $setViewValue method when you do so.

@runk
Copy link
Contributor Author

runk commented Nov 5, 2013

@quazzie there is alternative solution #4797.

I have a feeling that this trick with $setViewValue methods looks like a hack.. @matsko any thoughts?

@quazzie
Copy link
Contributor

quazzie commented Nov 5, 2013

@runk yeah that's maybe a nicer way to do it. But please add the tests.
The only thing i can complain about is that your solution uses 2 'watches'.

@runk
Copy link
Contributor Author

runk commented Nov 6, 2013

Yeah, sure @quazzie. It looks like that tests work fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants