Skip to content

Add changeHandler opt#10

Merged
quicksketch merged 2 commits intoquicksketch:masterfrom
fardjad:master
Dec 14, 2013
Merged

Add changeHandler opt#10
quicksketch merged 2 commits intoquicksketch:masterfrom
fardjad:master

Conversation

@fardjad
Copy link
Copy Markdown
Contributor

@fardjad fardjad commented Dec 14, 2013

I just added the changeHandler option.

It should be a function with the following signature:

changeHandler (timezoneName, countryName, offset)

which gets called upon timezone change.

That makes timezonepicker much easier to use on Android/JavaFX WebView or similar environments via a bridge.

Also I used uglifyjs with default options for minification. Let me know if there's a better minifier/configuration that I should use.

@quicksketch
Copy link
Copy Markdown
Owner

Thanks! Could this problem also be solved by #5? That would allow you to bind directly to the browser-level change events.

Otherwise, this looks like a good option to have, but it should also be documented.

@fardjad
Copy link
Copy Markdown
Contributor Author

fardjad commented Dec 14, 2013

Yes, but I think this one is more flexible than #5.

I already added the following lines to README.md

// A function to be called upon timezone change
// timezoneName, countryName, and offset will be passed as arguments
changeHandler: null

should I document it anywhere else?

@quicksketch
Copy link
Copy Markdown
Owner

Sure, I think that would be adequate. I've got a few other minor nitpicks that I'll add to the diff.

Finish changeHandler comments with a period.
Remove the whitespace in line 73.
@fardjad
Copy link
Copy Markdown
Contributor Author

fardjad commented Dec 14, 2013

Done.

quicksketch added a commit that referenced this pull request Dec 14, 2013
Added new option for specifying a handler on timezone change.
@quicksketch quicksketch merged commit 5815735 into quicksketch:master Dec 14, 2013
@quicksketch
Copy link
Copy Markdown
Owner

Super. Thanks!

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.

2 participants