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

Support to date, datetime and time inputs. #935

Merged
merged 10 commits into from
Dec 5, 2013

Conversation

volmer
Copy link
Contributor

@volmer volmer commented Nov 15, 2013

I changed DateTimeInput to support the new HTML5 form helpers that comes with Rails 4 (date_input, datetime_input and time_input) instead of rendering datetime selects, by explicitly passing the html5 option.

For example, this input:

<%= f.input :born_on, as: :date, html5: true %>

Will render something like:

<input id="user_born_on" name="user[born_on]" type="date" />

Classic datetime selects are still present for cases when the html5 option is not passed, in order to keep backwards compatibility.

Closes #844.

I changed `DateTimeInput` to support the new HTML5 form helpers that
comes with Rails 4 (`date_input`, `datetime_input` and `time_input`)
instead of rendering datetime selects:

For example, this input:
```erb
<%= f.input :born_on, as: :date %>
```
Will render:
```html
<input id="user_born_on" name="user[born_on]" type="date" />
```

Datetime selects are still present, but only when HTML5 compatibility
is disabled. It's also possible to render selects even with HTML5 on,
by setting the `html5` option to false.

If we wanted to use an old date select on the example above, we should do:
```erb
<%= f.input :born_on, as: :date, html5: false %>
```

Closes #844.
@@ -1,8 +1,14 @@
module SimpleForm
module Inputs
class DateTimeInput < Base
enable :html5
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to enable html5 here?

@rafaelfranca
Copy link
Collaborator

This will be a lot non backward compatible, but I wonder if we can make it more opt-in instead of opt-out.

@nashby @carlosantoniodasilva any ideas?

@nashby
Copy link
Collaborator

nashby commented Nov 15, 2013

@rafaelfranca yeah, I agree with opt-in. I would make html5: false to be default value.

@volmer
Copy link
Contributor Author

volmer commented Nov 16, 2013

@rafaelfranca @nashby Agreed. Datetime selects are now the default again.

@@ -19,6 +23,14 @@ def label_target
position = ActionView::Helpers::DateTimeSelector::POSITION[position]
"#{attribute_name}_#{position}i"
end

def use_html5_inputs?
if input_options.key?(:html5)
Copy link
Contributor

Choose a reason for hiding this comment

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

just input_options[:html5] should be enough here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@carlosantoniodasilva
Copy link
Member

I'm happy to bring html5 date/time fields to Simple Form, however I'm kinda afraid this will bring us issues with people upgrading.

Think about this scenario: I have html5 enabled and I'm happy with the current date/time selects, because the other (html5) tags were/are not currently well supported. After upgrading Simple Form, all my date/time fields are going to magically start using the new html5 tags because we changed to rely on the html5 flag (which is enabled in my app due to the other features I wanted).

Perhaps we should do it the other way around, be opt-in for these? Maybe we should provide a "migration configuration option" so that you can disable the date/time specific html5 fields instead of having to go figure the places you must pass in html5? Maybe we should provide a warning message or whatever to aid people migrating? We have a couple solutions, not sure what's best, but I'd prefer not to have my inputs changed due to a Simple Form update.

And great work here 👍

@volmer
Copy link
Contributor Author

volmer commented Nov 22, 2013

@carlosantoniodasilva Following @nashby's suggestion, In my last commits I've changed these inputs to render the new tags only when html5: true is explicitly passed to the input, so current users of HTML5 wrappers should not be affected when upgrading. I believe that's enough to help people upgrading, but sure we can improve the migration with an option with a different name and/or a warning message.

@rafaelfranca
Copy link
Collaborator

@volmer since it will not be the default we need to change the mapping table to point to date select again.

@volmer
Copy link
Contributor Author

volmer commented Nov 26, 2013

@rafaelfranca The mapping table was not changed, since the input class is the same.

@volmer
Copy link
Contributor Author

volmer commented Nov 26, 2013

@rafaelfranca Ah, got it. You mean the README instructions 😄 I'll fix that.

@volmer
Copy link
Contributor Author

volmer commented Nov 26, 2013

@rafaelfranca Done.

@rafaelfranca
Copy link
Collaborator

This seems good to me. Needs a rebase

@volmer
Copy link
Contributor Author

volmer commented Nov 27, 2013

@rafaelfranca 👍

@pcriv
Copy link

pcriv commented Nov 29, 2013

is this feature avaible yet on the master branch?

@volmer
Copy link
Contributor Author

volmer commented Nov 29, 2013

@pablocrivella not yet. @carlosantoniodasilva @nashby @lucasmazza is this good to go?

@lucasmazza
Copy link
Contributor

:shipit:

@pedrosnk
Copy link

👍

rafaelfranca added a commit that referenced this pull request Dec 5, 2013
Support to date, datetime and time inputs.
@rafaelfranca rafaelfranca merged commit 1071d8f into master Dec 5, 2013
@volmer volmer deleted the vs-html5-datetime-inputs branch December 5, 2013 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Equivalent of Input to Rails 4 date_field
7 participants