Skip to content

Refactor numeric inputs. Closes #758 #770

Merged
merged 5 commits into from Jan 3, 2012

5 participants

@haines
haines commented Dec 29, 2011

No description provided.

@justinfrench
Owner

Looks good! One concern I have is that we don't change any method names if possible, in order to provide as much backwards compatibility as possible for those that have subclassed NumberInput in their apps. I've had a quick eyeball, but if there's anything you can think of we need to review, that'd be great.

Similarly, removing the stringish class from the wrapper, while logical, creates a backwards compatibility issue for those that have styled against that class. From a ruby point of view, it's no longer Stringish, but from a CSS perspective, I think it is, so we should keep it (I'm okay with adding additional classes).

Also wondering if we should continue the ish naming scheme. Timeish, Stringish, Numberish?

@haines
haines commented Dec 30, 2011

Ruby-wise this is fully backwards compatible. As far as the naming scheme goes, I did think about Numberish but ended up going for Numeric simply because it sounded more natural to me. If you would rather be consistent, I don't mind.

@yabawock
Collaborator

+1 from me. Should close #728 as well if I'm right

@haines
haines commented Dec 30, 2011

@yabawock yep, so it does

@yabawock yabawock merged commit ab27e75 into master Jan 3, 2012
@jnimety

Why was this removed?

Owner

@haines can you join in here?

AFAICT limit is not applied to Numerics. I think this is a better solution, providing max/min is much more accurate.

@themactep

This makes it impossible to add a size attribute to number input controls. I believe 'size' is still a valid attribute, see DOM interface http://www.w3.org/TR/html-markup/input.number.html

According to MDN:

Starting in HTML5, this attribute applies only when the type attribute is set to text, search, tel, url, email, or password; otherwise it is ignored.

So while it might strictly be valid, it's not terribly useful. My bad for not getting that right in the comment, will fix when I'm back at my computer!

Have changed the wording from "not valid" to "does not apply" - see 4543f44

Can you remove this portion of code altogether instead? I don't see why you need to force-strip a valid attribute. The size attribute is intended to set the number of characters that can be seen at once, and it does apply to number inputs.

UPD: Seems Chrome does not respect the size attribute. It defaults to 20 and does not change size of the field whatever value I feed it. Firefox changes the field size according to the value. Weird.

Owner

@themactep If we can't remove it, or we're concerned that it might introduce an edge case regression, I think we can still accommodate you by checking if there's a key set:

defaults[:size] = nil unless defaults.key?(:size)

Would happily accept a patch with test coverage, otherwise please create an issue to track rather than this comment thread. I won't have time in the next few days.

The following content attributes must not be specified and do not apply to the element: accept, alt, checked, dirname, formaction, formenctype, formmethod, formnovalidate, formtarget, height, maxlength, multiple, pattern, size, src, and width.

http://www.whatwg.org/specs/web-apps/current-work/multipage/states-of-the-type-attribute.html#number-state-(type=number)

@mikz mikz deleted the numeric_inputs branch Mar 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.