Add HTML5 input[type="time"] helper #5715
Conversation
| def render | ||
| options = @options.stringify_keys | ||
| options["value"] = @options.fetch("value") { value(object).try(:strftime, "%H:%M:%S") } | ||
| options["size"] = nil |
There was a problem hiding this comment.
@rafaelfranca input[type="time"] doesn't have size attribute.
There was a problem hiding this comment.
Is this nilling out our own default size, or silently dropping a user-provided size?
Better to raise an ArgumentError if the user provided a size, if we really don't want it.
But better yet, does it matter? Most browsers show time input as a text field and respect size, so why not allow it?
There was a problem hiding this comment.
@jeremy It's nilling out user-provided size. I followed the style of other code in this case.
There was a problem hiding this comment.
Cool, ok. I think we're doing it wrong elsewhere, too! Looks good for now though.
There was a problem hiding this comment.
These code was removing the size since we added a default value to it. Since we removed the code that add this default value we can remove these lines. I'm fixing it now.
There was a problem hiding this comment.
I removed in the other places. We can remove here now.
There was a problem hiding this comment.
@rafaelfranca thank you. I removed this line too.
|
This no longer merges cleanly, and will need a rebase. |
|
@steveklabnik done. Thank you! |
| @@ -646,6 +646,32 @@ def test_date_field_with_nil_value | |||
| assert_dom_equal(expected, date_field("post", "written_on")) | |||
| end | |||
|
|
|||
| def test_time_field | |||
| expected = %{<input id="post_written_on" name="post[written_on]" type="time" value="00:00:00.000" />} | |||
There was a problem hiding this comment.
is the value required? I think that if we don't have a value it should be blank.
There was a problem hiding this comment.
@rafaelfranca you are right, the value isn't required.
Time is not blank in this case, it is 00:00:00.000.
|
Guys, do you have any new feedback? |
|
Could you squash the commits. I'll ask to @jeremy to merge it. |
|
@rafaelfranca done. It's just one commit now. |
Add HTML5 input[type="time"] helper
No description provided.