Skip to content

Conversation

@shekibobo
Copy link
Contributor

Adds XDSoft's DateTime picker as the combined_date_time_picker input for forms.

IRC prompting this PR.

Please try this out and see if there's any tweaks or suggestions to better implement this, or if you notice any bugs.

@shekibobo
Copy link
Contributor Author

This is ready for review, if anyone wants to try it out.

@shekibobo
Copy link
Contributor Author

Works as expected as a drop-in replacement for datepicker (accepts the same basic options, etc). Time format looks like 2014-03-17 14:50 by default, but can be overridden by overriding format on the datepicker_options and input_html/value on the combined_date_time_picker arguments.

Tests work (and fixed one spec that wasn't even running before).

@shekibobo
Copy link
Contributor Author

If this looks good, I might also consider changing the default datetime filter field to provide more precise time-based filtering. Thoughts?

@shekibobo
Copy link
Contributor Author

Figured out I was loading the datetime picker wrong. This one is meant to load whenever the page loads. I changed it so that it now loads whenever the page is loaded and whenever a has_many fieldset is loaded containing a combined_date_time_picker field. Now the field can properly accept options like mask: true.

@seanlinsley
Copy link
Contributor

Sorry I haven't gotten around to reviewing this :-( I should have the time to do it justice this coming week.

@timoschilling
Copy link
Member

Nice! But I think AA should not have to manny dependencies!

To make a activeadmin-xdan-datetimepicker gem could be a good way.

@seanlinsley
Copy link
Contributor

If we can make this completely replace the existing datepicker, I'm fine adding this as a required dependency. It's much more feature-complete than what we have now.

@shekibobo
Copy link
Contributor Author

I might be able to make that happen this weekend.

@shekibobo
Copy link
Contributor Author

IRC

@seanlinsley
Copy link
Contributor

Let me know if you need someone to bounce ideas off of 🐱

@shekibobo
Copy link
Contributor Author

At this point I really want to figure out a good way to make formats match between the input and the picker. That might mean figuring out a converter. It might mean splitting off another fork of xdan's repo with the formatter/parser replaced with one more Ruby-friendly. Unfortunately Javascript isn't my strong suit, so it may take some time to figure out on my own, but I plan on digging into it as soon as I get some time to dedicate to it.

@seanlinsley
Copy link
Contributor

IRT time formats, I've found that Y-m-d g:i A works well with Rails. Though it looks like this plugin doesn't support time zones at all 😦

@seanlinsley
Copy link
Contributor

I'd really love to have this as part of Active Admin. Do you have the time to rebase it?

@shekibobo
Copy link
Contributor Author

I can try to rebase it, but I'm not sure it's ready to be merged in yet. I probably won't have time to look at it today, but probably some time later this week.

@seanlinsley
Copy link
Contributor

No rush. Just wanted to make sure you're still around to work on this 🐙

@shekibobo
Copy link
Contributor Author

Yup, still here. Just not a lot of time to delve into AA source lately.

@shekibobo
Copy link
Contributor Author

Rebased with minimal issues.

Again, I don't really have a whole lot of time right now to look into this, but the issue of the date-time format should probably be addressed, and I'd be open to ideas on how we might solve that problem. I remember seeing someone post a conversion script (can't remember what issue, or if it was even here), and that might be worth exploring.

This also does not replace the current datetime picker, nor the form inputs. However, I think this would be a huge improvement to the form datetime inputs, since it would allow users to filter by a specific range of time within a day as well as by the date, and also solve the issue of searching with the same date in the start and end fields.

@shekibobo
Copy link
Contributor Author

On the other hand, we could just merge this in and start dealing with issues as they come. Maybe the concerns I have will be minor, but enough of a pain point for someone else that has time to tackle it before I do. 🐱

This will now go through the page and find all fields
which will use the combined date-time picker and set them up
on page load. It will also go through and find relevant fields
to set up whenever a has_many field is added.
@shekibobo
Copy link
Contributor Author

I've updated this to use the combined picker for date range filters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it is named CombinedDateTimePickerInput and not only DateTimePickerInput?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backwards compatability. There is already a datetimepicker.

On Sat, Nov 1, 2014, 9:43 PM Timo Schilling notifications@github.com
wrote:

In lib/active_admin/inputs/combined_date_time_picker_input.rb:

@@ -0,0 +1,28 @@
+module ActiveAdmin

  • module Inputs
  • class CombinedDateTimePickerInput < ::Formtastic::Inputs::StringInput

Why it is named CombinedDateTimePickerInput and not only
DateTimePickerInput?


Reply to this email directly or view it on GitHub
https://github.com/activeadmin/activeadmin/pull/3000/files#r19709171.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a DatepickerInput, but I can't find a DateTimePickerInput

@Fivell
Copy link
Member

Fivell commented Feb 12, 2015

@shekibobo , do you need any help ? This feature is very cool, hope it will be merged soon

@shekibobo
Copy link
Contributor Author

hey, sorry for falling off the planet for a while. I haven't really been working in ActiveAdmin for a few months now (actually working in Android these days), so I haven't really had a lot of time to focus on this.

I have been seeing more activity on the timepicker lately, and I've been meaning to look into it to see if new work would mean easier integration. I will try to update my gem repo for xdan-datetimepicker today, but if anyone else would like to take up this torch, I'd really appreciate it, but honestly I'm probably not going to have the free time to work past the challenges for this integration.

That being said, the README for the gem does include some examples on how to integrate the timepicker using both formtastic and simple_form, which I had mostly extracted from this PR and used in my own projects that used AA.

@Fivell
Copy link
Member

Fivell commented Mar 15, 2015

I continue working on this feature here https://github.com/Fivell/activeadmin/tree/timepicker

  • CombinedDateTimePickerInput renamed to DateTimePickerInput
  • fixed specs and cucumber features
  • undo deletion of DatePicker while DateTimePickerInput integration is not completed.

If somebody want's to play with it you can use this branch or plugin https://github.com/ActiveAdminPlugins/activeadmin_datetimepicker

Things todo before even thinking to merge to AA

  • restyle datetimepicker using AA styles, respecting sass variables and so on (this is where I need community help)
  • replace datepicker with date_time_picker (can be customised even for time picker )

@timoschilling
Copy link
Member

While @Fivell has released a integration as gem, I no longer see a reason for this PR

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.

4 participants