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

HTML5 range input support #417

Closed
justinfrench opened this issue Nov 1, 2010 · 17 comments
Closed

HTML5 range input support #417

justinfrench opened this issue Nov 1, 2010 · 17 comments

Comments

@justinfrench
Copy link
Member

Leverage validation reflection for min and max attributes?

@farnoy
Copy link
Contributor

farnoy commented Jan 25, 2011

I'm waiting for this feature and I want to contribute. How can I help implementing this?

@justinfrench
Copy link
Member Author

Hi, the simple answer is fork and patch! The slightly longer answer is that we're doing a bunch of refactoring in the v2 branch before I push ahead on any new features. There's stacks of value in someone figuring this out though — coming up with the test cases and checking the DSL fits, etc. Get started and we'll find a way to merge it in!

@farnoy
Copy link
Contributor

farnoy commented Jan 26, 2011

Ok, so I've made the fork and created that input. It creates generic rails input: range_field. It also uses reflection for range validation. I tested it with the following model:

class NumberContainer < ActiveRecord::Base
  validates_numericality_of :first, :greater_than => 0, :less_than => 6
  validates_numericality_of :second, :greater_than_or_equal_to => 1, :less_than_or_equal_to => 5
  validates_numericality_of :third, :greater_than_or_equal_to => -1, :less_than => 7
end

It works and generates allowed ranges of values for the input as following for those examples:

  • 1..5
  • 1..5
  • 0..6

I'm not however convinced, that this input is any useful, these are just sliders without legend, so tell me what you think. The code is here

@justinfrench
Copy link
Member Author

Looks like a great start. If you did an ||= on :in, this would allow people to supply thier own range at a view level:

options[:input_html][:in] ||= (range_start..range_end)

And :in is a lot like :collection, so it could be an option in the top namespace, allowing this:

f.input :foo, :as => :range, :in => (1..5)

In regards to markup, I think the right elements are there, but there's a few more attributes we could nail. :in is a Rails shortcut for min and max, so they could/should be allowed (maybe that's a great reason NOT to shift :in to the top level options).

There's also :step which instructs clients to only allow incremental values in steps of two, etc. So we could cater for that.

Finally, to smash it out of the park, I'm wondering if we could automatically build a :hint (if one is not set, not false, not already translated, etc) which describes the allowed values:

A number between 1 and 10
A number greater than 2
A number less than 100 in steps of 20
Etc.

This would hep users with clients that don't do much with the new elements.

You certainly don't have to do all that, but this is the place for the brainstorming and discussion :)

@farnoy
Copy link
Contributor

farnoy commented Jan 31, 2011

Oh, I see your point and I'll provide that :in and :step options. But I'll have to look up how to build a hint.

@farnoy
Copy link
Contributor

farnoy commented Jan 31, 2011

Tested new features with f.input :second, :as => :range, :in => 6..7, :step => 0.5 and it works perfectly.

@justinfrench
Copy link
Member Author

Brilliant! Test coverage would be great, and we can treat the hint stuff as a separate issue.

@farnoy
Copy link
Contributor

farnoy commented Feb 1, 2011

Ok, I'll write some tests too, but there's one more thing. I'm using validation_reflection itself when it's defined, because I've not seen anything other than reflection_for association in formtastic, is that correct? After that, as you've said, I'll wait with pull request for your v2 branch to clean up.

@justinfrench
Copy link
Member Author

reflect_on_validations_for seems to be what we're using elsewhere in the code, so you seem to be using the established patterns. I thought validation reflection was kind of "solved" in Rails 3 and didn't require a plugin any more, but the comments still hint at it, so who knows.

@farnoy
Copy link
Contributor

farnoy commented Feb 1, 2011

That's tested now and with this code I can implement some helper for validation that specifies range (and step?) options. It could be used in number_field_tag and similar.

@farnoy
Copy link
Contributor

farnoy commented Feb 1, 2011

I've made that helper and used it for :as => :numeric, which now prints rails's number_field instead of standard text_field. Take a look at new ValidationHelper (name may change) and changes to numeric field. #range_options_for can be now used to extract raw options taken from inline settings and validation_reflection to provide {:input_html=> {:in=>1..5,:step=>1}} for input. What do you think about that solution? :as => :numeric is better for end-user, because you see the actual value and your browser limits you, so you always pass validations.

@farnoy
Copy link
Contributor

farnoy commented Mar 10, 2011

How's refactoring by now? Are you close to the finish? I think this can be pulled without conflicts and gives numeric fields better support.

@sobrinho
Copy link
Member

farnoy, justinfrench are doing another refactoring.

please wait a little more.

@justinfrench
Copy link
Member Author

Ok, refactor branch is pretty smooth. I'll be merging into master pretty soon, so now is as good a time as any to get this in! The refactor branch is a pretty major diversion from v2, (class based inputs, not modules) so you may have to re-implement, but you should be able to copy the specs straight over and the new code base is ultra clean.

If you're out of time, send me a pull request against v2 and I'll try to port it over myself. If you want help groking the new code, sing out!

@farnoy
Copy link
Contributor

farnoy commented Apr 5, 2011

I will try to prepare it for the refactor branch and that shouldn't take long. I'll pull request my changes when I'm done and will describe everything.

@justinfrench
Copy link
Member Author

Awesome!

@justinfrench
Copy link
Member Author

This has been (mostly) implemented and merged into master. Remaining issues have separate issues created, so closing this!

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

No branches or pull requests

3 participants