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

Bring time input behavior in line with time_select when inputs are empty #730

Merged
merged 1 commit into from
Nov 15, 2011

Conversation

jslag
Copy link
Contributor

@jslag jslag commented Oct 28, 2011

Before this change, it wasn't possible to create an input of type 'time'
that didn't default to the current date. This made it impossible to
differentiate between a form submission wherein the user hadn't picked a
date, and one in which they'd picked midnight UTC.

Before this change, it wasn't possible to create an input of type 'time'
that didn't default to the current date. This made it impossible to
differentiate between a form submission wherein the user hadn't picked a
date, and one in which they'd picked midnight UTC.
@sobrinho
Copy link
Member

Makes sense for me. @justinfrench?

@justinfrench
Copy link
Member

Does this change default behaviour from "if there's no time on the model, make it 'now'" to "if there's no time on the model, show nothing"? I'm not necessarily against that change, but we should be conscious of it.

@jslag
Copy link
Contributor Author

jslag commented Oct 28, 2011

The way I would put the current behavior is "if no time is entered in a formtastic time input, save 2000-01-01 00:00 UTC to the database". This change results in that scenario leaving the field nil, which is desirable if you ever want a time field to be optional.

@justinfrench
Copy link
Member

"save … to the database" is too implementation specific, so let's stick to what the user sees on the form — if there's no time present on the attribute, what did the user see, what will the see going forward?

@jslag
Copy link
Contributor Author

jslag commented Oct 28, 2011

You're asking about an edit form, right? Going forward, the user sees blank values for the time dropdowns when the field is nil, same as they would for a new object

@sobrinho
Copy link
Member

I think we should not set "default values".

Let think about optional date time field.

If user want to keep this field blank, they should change the value because the form render the current time by default.

I guess if developer want to set the current time as default, he should do something like that on controller:

@account = Account.new
@account.create_at = Time.current

That's the default behavior of rails, I guess.

@justinfrench
Copy link
Member

@sobrinho, totally agree, but anything that changes the default behaviour of Formtastic should be at least well documented in the release notes, so I'm trying to figure out what the user-facing change is.

@jslag
Copy link
Contributor Author

jslag commented Oct 28, 2011

Sounds like we're basically on the same page, then.

The release notes could read something like "Changed time input so that blank values aren't persisted as midnight"

Would it also make sense to point out that this change makes the formtastic behavior consistent with ActionView's time_select with include_blank? Eg.

time_select foo, bar, :include_blank => true

justinfrench added a commit that referenced this pull request Nov 15, 2011
Bring time input behavior in line with time_select when inputs are empty
@justinfrench justinfrench merged commit 45ec7a7 into formtastic:master Nov 15, 2011
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