Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for NSFormatter #306

Merged
merged 1 commit into from
May 31, 2016

Conversation

ziogaschr
Copy link
Contributor

All the credits for this pull request are for @bhirt-bpl who made the initial PR #77 and also to @fwhenin who made an attempt #195 to rebase it 4 months ago.

I rebased all the work from @bhirt-bpl at current master and also removed the extra commits/features @fwhenin added on his PR.

  • Added formatter related properties and methods to XLFormRowDescriptor
  • Updated XLFormTextFieldCell to use formatters when specified.
  • Created example page for formatters, FormattersViewController, and linked from OthersFormViewController
  • Added SHSPhoneComponent to Podfile to demonstrate a phone formatter on FormattersViewController.

It will be nice if you could accept this Pull Request soon. Thanks

@ziogaschr
Copy link
Contributor Author

Hello,
I have just synced the PR with the master branch.
Are you planning to include this PR?
Is there something that's missing that I can add or anything that should be changed?

At least then I could help so as this PR is being merged at some point.

Thanks a lot for the great work that you are doing

@mtnbarreto
Copy link
Member

@ziogaschr I will review the code and get back to you with my comments on this. We should have to figure out a elegant way to expose these properties (i don't like the idea of exposing them as XLFormRowDescriptor properties and methods).

Perhaps implementing all these stuff inside XLFormTextFieldCell class and setting up the feature using cellConfigAtConfigure XLFormRowDescriptor property keeps the XLFormRowDescriptor interface simple adding the ability to use NSFormatters.

My point is that NSFormatter usage is a edge case scenario and i don't want to expose more properties inside XLFormRowDescriptor to handle it because it will make XLForm harder to use.

Anyway let me think a little bit about it.
Regards,
martin

@allaire
Copy link

allaire commented Jun 1, 2015

Any news on this?

@Romaric5M
Copy link

Seems to be a good solution for my issue and more.
Is there any news on this request ?

Thanks

@fwhenin
Copy link

fwhenin commented Jul 27, 2015

so I ended up adding some personal stuff in the forked version of this Pod, I'm now trying to upgrade to 3.0 and realized that this wasn't added, might have to re-fork again and add the formatter again in my own version of the app

@fwhenin
Copy link

fwhenin commented Jul 29, 2015

@mtnbarreto How would you do something like entering a currency. Usually the user would enter a decimal value, but it's nice to have a '$' ahead of it along with making sure it's always 2 decimals...or how about a number format. making "1234567890" into "(123) 456-7890". I don't believe that THOSE would be edge cases

@Romaric5M
Copy link

Any news ?
Thanks

@ziogaschr
Copy link
Contributor Author

I just rebased my branch with your master for anyone interested using it.

@cerupcat
Copy link

Any update on merging this? What's the case against it? It seems fairly common use case to format certain text.

@ziogaschr
Copy link
Contributor Author

ziogaschr commented Apr 24, 2016

@mtnbarreto have you thought on it? Let me know of which you think the best way for such a feature will be, so as I implement it in May. I wouldn't like to work on something that will not be merged. So please advice me on the best possible way for such a thing.

Thanks, Chris :)

@ziogaschr
Copy link
Contributor Author

Any luck on getting this feature merged or even ask me to redesign. I have to merge my fork again from version 2.2.0. It will be nice if I can use your repo directly :)

@mats-claassen
Copy link
Member

Hi @ziogaschr . I will merge it if you can rebase it onto the latest master version.

@ziogaschr
Copy link
Contributor Author

Hi @mats-claassen. Thanks for the quick response. I merged it with master can you accept the merge?
Let me know if you want me to change something. Thanks

@mats-claassen mats-claassen merged commit de86c5e into xmartlabs:master May 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants