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

Fix 24h display when showMeridian is false. #209

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from

Conversation

astjohn
Copy link

@astjohn astjohn commented Feb 10, 2014

This pull request adds a simple padding method for use when showMeridian is false and elsewhere. I added one extra spec for testing. 24 hour times should now have double digits on display. This should fix #208.

I also noted that the pull request #203 did not actually set the display, only the underlying values. In my opinion, this pull request has a cleaner and easier to read solution.

@dietherw
Copy link

Thanks! This was just causing a problem in our input validation.
So far this fix is working fine. Hopefully it'll be in the next release soon.

@astjohn
Copy link
Author

astjohn commented Feb 11, 2014

@dietherw: No problem. It was definitely needed!

@comtom
Copy link

comtom commented Feb 24, 2014

Did the trick. Awesome work thanks! I noticed that when the field is empty, and I change hours (now hours are working great) in minutes field I get '0' when I should get '00' this happens until I change minutes value. I know, this is a minor issue, but I'm in the obligation to report ir.
@astjohn Thanks for your time. Kudos

@eleumik
Copy link

eleumik commented May 9, 2014

nice, better have both options as some locale use always 2 digits ( fr, de..), some not (it, nl..)

eleumik added a commit to eleumik/bootstrap-timepicker that referenced this pull request May 9, 2014
@astjohn
Copy link
Author

astjohn commented May 10, 2014

@eleumik, Regarding 24 hours times: I think it's best to follow standards. In this case, ISO 8601 specifies that 24 hour times should always be zero padded.

@eleumik
Copy link

eleumik commented May 10, 2014

@astjohn In my opinion we are talking about 2 distinct things: sending user input/localized data and sending a format, both with their use case. Sometimes (maybe often) one may have the possibility of sending a format but only when scripts are active; one cannot ask a user to type ISO8601 for a date-time, for time alone I admit, you might ;-) however in some cultures they might be not be happy to type 24-hours time.

I use the picker to send localized data, same as appears in the input. The reason is that is there are no scripts enabled the widget degrades gracefully to something that can be used: a standard input type='text' in which the user may type his time in his/current language. I read localized data on the server in both cases. For this reason I said is better to have an option.

Btw just yesterday I updated a version of the picker since I had already this case in mind, may be inspired by this same thread, so I made an option to always send ISO and display localized, in this way the user can use AM/PM but always send 24H ISO. I would appreciate if you test whether it suits your use case, there should be also some other bug fixes, I am waiting to see if I can merge here, I have tried to not change the original default behavior.

You may test the two digits here [data-two-digits-hour='true'] http://jsbin.com/xahux
and the ISO send [data-submit-mode='iso' or 'iso-auto' (without seconds if not displayed)] here http://jsbin.com/qiyeb/2

A warning: I am linking the jscript 'live' from my github project https://raw.githubusercontent.com/eleumik/bootstrap-timepicker/gh-pages/js/bootstrap-timepicker.js and only Mozilla still allows that, other browsers may not work.

Let me know if I can help, sorry for being so long but I took the occasion to summarize what I am doing, thanks.
Bye

https://github.com/eleumik/bootstrap-timepicker

http://jsbin.com/kexil/6 (localization showcase)

peichhorn-netgo pushed a commit to netgo-software/bootstrap-timepicker that referenced this pull request Aug 27, 2015
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.

24hr format issue
4 participants