-
Notifications
You must be signed in to change notification settings - Fork 1k
12 hour format, force alt field to always be UTC #492
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
Conversation
Hey Vaughn, Thanks for your work here. A couple notes.
Parsing Time Option 1: Parsing Time Option 2: Do you have any thoughts or suggestions? I believe leaning more on timeFormat than a separate ampm option is the way to go, I just want to make sure all ends (parsing and outputing) are on the same page before pushing a live version. |
Hey Trent, actually, ampm is NOT used for parsing, it's only used for controlling the display of the sliders, which IMO is and should be a separate setting from the control of the regular and alt inputs. the ampm setting no longer affects the internal time values, that is, it never affects/changes the "hour" variable, rather that's left to a 12-24hr conversion function when invoked by using the new HH format specifier I think the current situation is actually good because now there are three different discreet and decoupled controls for 12 vs 24 hour format
and they can all be independent and different if desired which I have found can be a problem - this is in fact currently a problem in another "competing" plugin - data format should, in the ideal case, control/force/dictate display format as little as possible and I consider the formats of the input boxes to be much more "data" oriented than the format of the slider I guess it might be fair to say that one would never set timeFormat to use the 12 hour format and then have the sliders display 24 hour format, however I have run in to the the reverse problem already, and even though it may be an unlikely instance, I think the benefits of decoupling are worth leaving this as is If you wish to refactor because you have ampm coupled with some parsing, I would encourage you to move to a slightly different parameter that's meant to be interface-display only, aka only controlling the display of the sliders, and probably call it "use12HourInterface" or some such. |
and by the way, I think I do understand what you mean in regards to parsing - for now I am getting dates from Rails and they come in the ISO 0000-00-00T00:00:00Z format which is easily/automagically acceptable by JavaScript's Date object constructor and I personally have been using .datepicker("setDate", someDateObject); so I don't really have to worry about parsing from an input value="" attribute if you want to be able to pull in a string from the value="" attribute of the input box to populate the Timepicker instance with time values, then yeah I think parsing will need to be worked on here |
On the topic of ampm option, how about this alternative, would be much more consistant (Just bouncing ideas around): A "pickerTimeFormat" option, similar to timeFormat and altTimeFormat, this one would be used for display inside the picker and depending on it containing H or h it would handle the sliders formatting. It could default to timeFormat, and this way users can adjust how they like inside the picker? |
yeah I think that's a fine idea - I think it would also go well towards implementing another feature I personally see as semi critical which is that I think it would be ideal to allow the datetimepicker widget to hide all normal/related text-input fields and permanently display itself directly in the DOM, this as opposed to being hidden until the popup is triggered by a click the anytime picker lets you do this and I think it's a good feature and would be of high value to implement having a pickerTimeFormat option would work well with a feature like that |
some more thoughts as per your original comment about parsing options 1 and 2 I think it's safe/sane to say that the timeFormat to use when parsing a given as for whether to try parsing naturally with JS first, we could have a flag such as "tryToPlayNice" maybe - when set to true, it will try to just parse the date string via alternatively we may even wish to add a few more discreet options, "parseDateFormat", "parseTimeFormat", "altParseDateFormat" etc. respectively which would mirror dateFormat and timeFormat this would possibly also require an option such as altParseSeparator |
Ok, So my gameplan looks like: Input
Output
|
12 hour format, force alt field to always be UTC
I merged your changes to the dev branch. I looked at a few other language's timeFormat (php, python, coldfusion, etc..) and H commonly refers to 24 hour clock, and h refers to 12 hour clock, so I swapped those out. Added pickerTimeFormat and dropped ampm options. I added an internal method useAmpm(timeFormat) to detect when to use ampm from the timeFormat (it looks for h and t to be present). There is also a "strict" option which defaults to true for parsing. When false it will try new Date, if it fails it will try strict, else throws an error. |
cool, sounds good - I look forward to the rest of it thanks |
I've searched quite some time to fix this issue for ampm display settings. On a webapp I'm working on, the users can switch for 24h display or 12h ampm. So the ampm option is gone .. ? I fixed it, thanks to this post, to the following code: Only thing what is a bit pity, is the documentation of the datetimepicker. I love all of your work and can only encourage it. If you need help for documenting all of this, just pm ;) |
Hey Gunky, The ampm option is gone and pulls from the timeFormat like you have there. Now all support pulls from each timeFormat, pickerTimeFormat, altTimeFormat. The docs need to be redone sometime soon, but for the time being is there a particular item missing that needs to be added? |
Hi Trent, Well, like the ampm option which was deleted, I never found anything of it, except in this post. Would be handy if developers know how they can implement ampm setting :) |
Here is one fix and one feature
12 hour formatting option
The fix addresses issue(s) #459 and #479
to use the fix simply use a capital "H" instead of a lower case "h" in your time format specifier to force usage of 12 hour time format, "HH" with leading zero and "H" without, for example. The
ampm
option now only affects the process of time picking and not the resulting output. This will, for example, display times in 12 hour format, but the time picker should remain in 24 hour formatnot that you'd want this ever, but the feature works the other way as well - so of course, you can still use lower case "h" to get a nice display format on the frontend and a more easily parsable format for the backend
working demo: http://jsfiddle.net/SyNCp/1/