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

Allow min/max dates to be set on datepicker input #2578

Merged
merged 1 commit into from Nov 4, 2013

Conversation

shekibobo
Copy link
Contributor

Allows :max_date and :min_date options to be passed to :datepicker input.

f.input :starts_at, as: :datepicker, datepicker_options: { min_date: "2013-10-8", max_date: 3.days.from_now.to_date }
f.input :ends_at, as: :datepicker, datepicker_options: { min_date: 3.days.ago.to_date, max_date: "+1W +5D" }

@seanlinsley
Copy link
Contributor

That's because the date filter uses FilterDateRangeInput

Sorry, I initially thought you were setting up a filter. 😓

@seanlinsley
Copy link
Contributor

The problem is that the input_html_options is called multiple times by Formtastic. You should use Hash#slice instead of Hash#extract!

@shekibobo
Copy link
Contributor Author

Fixed and updated. Thanks @daxter!

range_opts = options.slice(:min_date, :max_date)
# convert the min and max date values to strings if they aren't already
range_opts.merge!(range_opts) { |k,v| v.to_s }
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I've never seen Hash#merge use a block before; what's happening here?

Are you sure that it's necessary to coerce the dates into strings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It basically maps all the values in the hash, keeping them on their
respective keys. Kind of a Hash#map, except it stays a hash. If I don't
coerce to strings, the data values show up as ""2013-10-16"". The double
quotes are weird, and they cant be read by datepicker.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's clearer to just iterate over it:

def datepicker_options
  options = options.slice :min_date, :max_date
  options.each{ |k,v| options[k] = v.to_s }
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I can change that in the morning.

@shekibobo
Copy link
Contributor Author

Tested this out locally, works with the button panel and the date ranges, though the button panel shows up has disembodied buttons. They show up, but they should probably have some kind of background on them.

@shekibobo
Copy link
Contributor Author

Is there anything holding this back from being merged in? Or is it just an issue of time to test it out?

@seanlinsley
Copy link
Contributor

Yeah, I've been rather scatterbrained lately 🐈
I'll test this out tonight to see if we can get it merged

it 'should generate a datepicker text input with data min and max dates' do
body.should have_tag("input", :attributes => { :type => "text",
:class => "datepicker",
:name => "post[created_at",
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, how is this test passing? The name parameter is missing a ]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's pretty weird. Fixed it below. Maybe the matcher is really forgiving or was able to infer the right name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm betting this would still pass if it was "post[creat". Just string matching on the HTML output.

@shekibobo
Copy link
Contributor Author

Actually I bet it's using string matching on the html output. Technically it matches correctly, just not completely.

@seanlinsley
Copy link
Contributor

Looks good to me, but can you squash these commits?

@seanlinsley
Copy link
Contributor

I just tried checking locally to see if merging #2645 would prevent your PR from being able to be automatically merged, and it worked fine. But the GitHub UI is telling me that this can't be automatically merged, so I guess that means that GitHub is incapable of doing recursive merges.

Could you also rebase this on master? 😅

Allows :max_date and :min_date options (or other jQueryUI datepicker
options) to be passed to :datepicker input.

```ruby
f.input :starts_at, as: :datepicker, datepicker_options: {min_date:
"2013-10-8", max_date: 3.days.from_now.to_date}
f.input :ends_at, as: :datepicker, datepicker_options: {min_date:
3.days.ago.to_date, max_date: "+1W +5D"}
```
@shekibobo
Copy link
Contributor Author

Actually, just noticed I'm no longer converting v.to_s... it still works for you, right?

@shekibobo
Copy link
Contributor Author

I guess it must. The spec is passing. I guess maybe I don't need to coerce Dates anymore? Been a while since I touched this one, but if you checked it out, then it should be good to go.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) when pulling 33b1f6c on shekibobo:datepicker-advanced into 3db28da on gregbell:master.

@seanlinsley
Copy link
Contributor

Given:

f.input :foo, as: :datepicker, datepicker_options: {
  min_date: "2013-10-8",
  max_date: 3.days.from_now.to_date
}

I get:
screen shot 2013-11-03 at 9 25 31 pm

Wait, what? Thursday? Shouldn't it be Wednesday?

3.days.from_now.to_date # => Thu, 07 Nov 2013

Hmm.

So it seems to be working without the to_s

Though is it me or is the font size larger for the disabled dates?

@seanlinsley
Copy link
Contributor

Meh, that can be dealt with later. Thanks for creating this PR :]

@shekibobo
Copy link
Contributor Author

Daylight savings time?

Looks like they're a different font weight and smaller, since they're underlined.

Also considering adding this to the docs on forms:

DatePicker

ActiveAdmin offers the datepicker input, which uses the jQueryUI Datepicker.
The datepicker input accepts any of the options available to the standard
jQueryUI Datepicker, e.g.

form do |f|
  f.input :starts_at, as: :datepicker, datepicker_options: { min_date: "2013-10-8", max_date: 3.days.from_now.to_date }
  f.input :ends_at, as: :datepicker, datepicker_options: { min_date: 3.days.ago.to_date, max_date: "+1W +5D" }
end

Thoughts?

seanlinsley added a commit that referenced this pull request Nov 4, 2013
Allow min/max dates to be set on datepicker input
@seanlinsley seanlinsley merged commit fcf09c0 into activeadmin:master Nov 4, 2013
@seanlinsley
Copy link
Contributor

Yeah, we should definitely have this in the form docs.

@shekibobo
Copy link
Contributor Author

Documentation in #2653.

@seanlinsley
Copy link
Contributor

yep :]

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.

None yet

3 participants