insin / react-maskedinput

Masked <input/> React component
http://insin.github.io/react-maskedinput/
MIT License
731 stars 197 forks source link

Added fixes from the PR board. #53

Closed martyphee closed 8 years ago

martyphee commented 8 years ago

Add support for updating the mask. Fixes #8.

Previously the cursor would go to the end of the input if you changed the mask or re-created the MaskedInput field. So, if you have a user typing 34 and then switching the mask to 1111 111111 11111 the cursor would be positioned to the end: 34__ ______ _____^.

This PR takes suggestions from PR #9.

iamdustan commented 8 years ago

what behavior is this intended to change or modify?

martyphee commented 8 years ago

@iamdustan I reopned this PR. Looks like the suggestions from the other issues/pr's fixed the issue.

iamdustan commented 8 years ago

cool. how extensively did you test this, btw?

iamdustan commented 8 years ago

could you update your commit message to describe the code change more clearly? E.g.

Add support for updating the `mask`. Fixes #8
martyphee commented 8 years ago

cool. how extensively did you test this, btw?

Right now I'm testing our use case which is rebuilding our checkout pages. I can add it to other fields in the checkout flow.

martyphee commented 8 years ago

@iamdustan Do you think we'll be able to get this merged on Monday?

iamdustan commented 8 years ago

Could you add a test case or two and an example of this to the demo?

That’ll make it easier to test out and verify it doesn’t regress in the future. Thanks!

martyphee commented 8 years ago

Fair enough. Not sure how to do the test case. Need to see where the cursor ends up I think.

On Sun, May 22, 2016 at 7:35 AM, Dustan Kasten notifications@github.com wrote:

Could you add a test case or two and an example of this to the demo?

That’ll make it easier to test out and verify it doesn’t regress in the future. Thanks!

— You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub https://github.com/insin/react-maskedinput/pull/53#issuecomment-220830130

martyphee commented 8 years ago

I updated the demo and added a test. Let me know what you think.

On Sun, May 22, 2016 at 8:00 AM, Martin Phee martyphee@gmail.com wrote:

Fair enough. Not sure how to do the test case. Need to see where the cursor ends up I think.

On Sun, May 22, 2016 at 7:35 AM, Dustan Kasten notifications@github.com wrote:

Could you add a test case or two and an example of this to the demo?

That’ll make it easier to test out and verify it doesn’t regress in the future. Thanks!

— You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub https://github.com/insin/react-maskedinput/pull/53#issuecomment-220830130

iamdustan commented 8 years ago

awesome. Thanks for adding that demo and test case, @martyphee. I’m on PTO a lot this week so won’t be able to test it myself until next week. I’ve asked @jquense to give it a look at his earliest opportunity to get this shipped. Based on his work inside React’s DOM event handling I trust him more than me anyway :)

martyphee commented 8 years ago

I fixed up the formatting. @jquense if you have time to review/merge/push that would be great. I have a outstanding PR in our project which depends on this change.

Thank you!

jquense commented 8 years ago

LGTM, can you please squash the commits and change the message per: https://github.com/insin/react-maskedinput/pull/53#issuecomment-220656793

martyphee commented 8 years ago

Done.

martyphee commented 8 years ago

@jquense When would you be able to do a release to npm? Thanks for the help.

jquense commented 8 years ago

unfortunately no, I don't have publish rights to npm sorry!

insin commented 8 years ago

I just added monastic.panic as an npm owner - does anyone else need/want access?

martyphee commented 8 years ago

@insin I only need it to get this published. I might contribute more in the future. martyphee

jquense commented 8 years ago

ok published at 3.2.0

martyphee commented 8 years ago

Awesome! Thank you!

On Tue, May 24, 2016 at 2:21 PM, Jason Quense notifications@github.com wrote:

ok published at 3.2.0

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/insin/react-maskedinput/pull/53#issuecomment-221369904