First attempt with date and time pickers. First patch then a zip file with files to put in a "picker" folder.

This might be better off as a separate module where we can get input from the whole community.

The pickers are far from perfect, there are some annoyances, but it works fine if you use it as expected...

* I'm sort of guessing with the timezone stuff.
* Some untranslatable JavaScript strings (I think the datePicker should be possible to translate as well)
* It is a lot of files, there is the CSS aggregation but no JS aggregation in Drupal 5, so we might want to do something about it.

When you test, write $("#advpoll-dates").show() in Firebug or remove #advpoll-dates from line 47 in timePicker.css to see the magic.

Comments

anders.fajerson’s picture

Zip is not allowed and I don't know how to add new files when making a patch. Email to the rescue. If anyone else except Chris want to test this, contact me :)

ChrisKennedy’s picture

Yeah it's really dumb that zip files aren't allowed. You can just change the extension to ".zip.txt" or something that is allowed though.

anders.fajerson’s picture

StatusFileSize
new19.13 KB

...

ChrisKennedy’s picture

Component: User interface (CSS, Images, etc.) » Code

I'm getting this error in Firefox:

Error: $("#advpoll-startdate").val(start.match(/\d\d\d\d-\d\d-\d\d/)).change is not a function
Source File: advpoll-form.js
Line: 95

Minor code review stuff:

Missing a closing angle bracket at '#suffix' => '</div'.

Need a space at if(end) { and if($(this)[0].checked) {.

true -> TRUE

Some extra spaces around the string concatenation should be removed.

Eventually the variable names should be written out in setDateTime().

anders.fajerson’s picture

Component: Code » User interface (CSS, Images, etc.)

BIG mistake. I just realized I coded this whole thing with a recent version of jquery in my misc folder (I needed it for another project). If you do want to test this, use the jQuery update module: http://drupal.org/project/jquery_update

anders.fajerson’s picture

Component: User interface (CSS, Images, etc.) » Code

Aaah, you found out...

ChrisKennedy’s picture

I think the problem is that you're chaining off of the val() which doesn't return the object. If you reverse the .change() and .val() order it works (and looks quite sweet).

ChrisKennedy’s picture

A few more things:

1) If it needs an updated jquery we should check that jquery_update is enabled before enabling the date and time pickers.
2) Yeah it is kind of slow to request that many javascript files. The best solution would for us to do the js aggregation (via a script or by hand if needed), add to cvs, and then include the aggregated file. Once we shift to D6 we can then remove the manual aggregation.
3) The UI placement is off a bit in IE7 - not horrible but not that great.
4) "Use end date" can definitely be passed as a translated string. Since "Date1 to Date2" will be hard to translate, what about just using the "Start date:" and "End date:" inline labels?

anders.fajerson’s picture

Status: Needs review » Needs work

So you think we should push this for Drupal 5 even if it requires the update module?

ChrisKennedy’s picture

Yeah I think it is a great UI addition, so at long as it degrades gracefully it will be a nice incentive to use jquery_update if they aren't already. Then for D6 we will be ready to go.

More for the TODO list:
* Need to use the Drupal setting for first day of the week: variable_get('date_first_day', 0).
* For the timezone I think something like this would be cleaner: $user->timezone? $user->timezone : variable_get('date_default_timezone', 0).

ChrisKennedy’s picture

Ah I just realized that we do true/false in JavaScript rather than TRUE/FALSE. Ignore that comment.

anders.fajerson’s picture

StatusFileSize
new5.9 KB

New picker (http://marcgrabanski.com/code/jquery-calendar/), fixes most of the issues I had with previous one (except the IE position bug which is related to Drupal/Garland somehow, not researched yet). Even keyboard support.

anders.fajerson’s picture

StatusFileSize
new16.95 KB

And the files.

anders.fajerson’s picture

StatusFileSize
new6.31 KB

Added and renamed labels. No description support yet (don't know if we really need it, complicates things it seems).

anders.fajerson’s picture

StatusFileSize
new1.38 KB

Updated CSS file. Place in "pickers" folder.

anders.fajerson’s picture

Darn it. The jQuery calendar is not under GPL (CC strangely enough). Note to self: always check header. I'll see if can get him to dual-license it, but if not we probably have to go back to datePicker.

anders.fajerson’s picture

On track again, the calendar is now GLP, thanks Marc Grabanski! It uses V3 but that should be ok(?):

Copyright (c) 2007 Marc Grabanski (http://marcgrabanski.com/code/jquery-calendar)
Dual licensed under the GPL (http://www.gnu.org/licenses/gpl-3.0.txt) and 
CC (http://creativecommons.org/licenses/by/3.0/) licenses. "Share or Remix it but please Attribute the authors."
anders.fajerson’s picture

StatusFileSize
new11.67 KB

* Added firstday support
* Reverted back to use no animation for the calendar as it caused a strange bug when used (I used speed: 1 to work around an Opera bug). When startdate and endate fields was manually cleared it was never updated to the hidden date fields. I don't want the animation anyways so lets hope the Opera bug is fixed soon.
* Re-introduced radios to get a cleaner interface. I tried with a complete JavaScript implementation but settled on doing it serverside which works much better with less code. Active status is now set to "2" if duration is specified, do we want to "normalize" it to "1" when saving/updating the node?

Todo:
* Check for jQuery Update module
* Your timezone code returns 7200, how is the best way to convert it to +0200?
* Labels/descriptions could probably be improved.

Known issues
* Position in IE is still off (I think we should release this anyway)
* The Opera bug mentioned above

ChrisKennedy’s picture

You can divide the timezone by 3600 to get +2 GMT (convert it from seconds to hours).

anders.fajerson’s picture

Status: Needs work » Needs review
StatusFileSize
new11.73 KB

The problem is that I need it in the format +0200. I'm not sure but looking at http://api.drupal.org/api/function/format_date is seems a bit complicated. Could we do it my way? I can add some more comments to it...

Added check for jQuery update and fixed a typo in firstDay.

ChrisKennedy’s picture

Ahh gotcha... yeah I see now how format_date makes more sense.

ChrisKennedy’s picture

Status: Needs review » Needs work

Needs a re-roll before any functionality reviewing can be done; coding style will eventually need work as well.

anders.fajerson’s picture

This effort will most likely be synced with the date_popup module: http://drupal.org/node/157127. (Note to self: always search search search).

ChrisKennedy’s picture

Hey do you think we can get a workable date UI solution for 1.0? It doesn't need to be perfect, but right now it's kind of a pain to have to type in the whole date manually.

anders.fajerson’s picture

Priority: Normal » Critical
Status: Needs work » Active

Can't promise anything, but yes we should and I think it's perfectly doable, be it date module, an own solution or jstools (given in prefered order).

ChrisKennedy’s picture

I would prefer to avoid Date.module if at all possible. We don't want to get into a VotingAPI situation where it holds us back more than its minor features are worth. Plus Date.module seems to use jstools for its javascript UI anyway, so we might as well keep control of our date fields and use jstools for the interface. Even then it would be better to create our own implementation and not be dependent on jstools to fix bugs. It doesn't seem to be maintained particularly well.

Isn't the code in #20 pretty much ready to go? If we drop the time UI and just try to get the date UI added, will that make things easier?

anders.fajerson’s picture

I think it's a difference here from VotingAPI. Date API and date_popup would not be required so they won't stop us from touching are own code. As with jstools we would do a module_exist().

The way date_popup splits date and time with PHP instead of JS is much more robust. We could of course do that as well but it seems like a lot of duplicate effort re-writing this. When I put a lot of time into this previously I really felt that this was something that would be best abstracted away form advpoll so it could be of benefit for other module developers and users.

I tried date_popup earlier today. It had some issues (calendar display and saving dates - try the author date field or change "textfield" to "date_popup" for end_date or start_date). But if we can get Karen to work with us making it stable in the near future I believe that is our best option.

anders.fajerson’s picture

Status: Active » Needs review
StatusFileSize
new1.04 KB

date_popup (HEAD in date module) was just updated and it seems to work pretty well. There are some styling issues and I still prefer my time picker (http://labs.perifer.se/timedatepicker/), but this is a good start and I still think it's our best option.

This patch just enables the pickers to start playing with it. We need do some styling but that could even go in in a seperate patch. I also just submitted this issue to the date project which should make it possible to rename the labels easily: http://drupal.org/node/201432

ChrisKennedy’s picture

Status: Needs review » Needs work
StatusFileSize
new6.38 KB

Sweet, KarenS fixed the PHP4 bug that was causing problems on my server, and it looks great.

Attached is what the basic form elements look like for me. I am mainly interested in removing the unnecessary "Date:" labels, and fixing the description, before committing. Personally I prefer YYYY-mm-dd for default date formatting (it's easier for sorting, automated searches, etc.), but I can ignore that point if it's too hard to change. Ultimately it seems like a superb UI improvement.

Minor: need a space after IF in the patch itself.

anders.fajerson’s picture

StatusFileSize
new1.48 KB

Seems like we have to dig a bit deeper in date_popup. Looking at how the author date is implemented this patch might be the better approach, need to look at it some more.

About the date format, using the patch in #28 the default short date format (admin/settings/date-time) is respected, which I saw as one of the big benefits of using date_popup. On the other hand, this patch defaults to YYYY-mm-dd. But I would think that's a bug.

Not sure what you mean about "easier for sorting, automated searches, etc.".

In my tests date.css is not initiated. You must have some other date related content on the page (e.g a calendar in a block?). So I don't have them inline, but the screenshot looks good :).

We need http://drupal.org/node/201432 to get in if we want to fix the labels without using form_alter (seems a bit silly to form_alter your own elements...)

ChrisKennedy’s picture

Ah using the default short date format does sound better, nevermind on that point.

pomliane’s picture

Status: Needs work » Closed (won't fix)

This version of Advanced Poll is not supported anymore. The issue is closed for this reason.
Please upgrade to a supported version and feel free to reopen the issue on the new version if applicable.

This issue has been automagically closed by a script.