Skip to content

Add number widget and expose user attributes #83

wants to merge 2 commits into from

2 participants

tellnes commented Oct 30, 2013

I can split this into two pull requests, but I did not find it necessary.

@ljharb ljharb commented on the diff Oct 30, 2013
@@ -26,11 +26,15 @@ var test_input = function (type) {
test.equals(forms.widgets[type]().formatValue('hello'), expectedValue);
test.strictEqual(forms.widgets[type]().formatValue(false), null);
+ test.deepEqual(forms.widgets[type]( { min: 1 } ).getUserAttrs(), { min: 1 })
ljharb added a note Oct 30, 2013

why is this test (and the resulting addition of getUserAttrs) necessary? Could you test the generated HTML instead of inspecting the innards of the widget?

I'm hesitant to expose things on components because it inhibits future refactoring.

tellnes added a note Oct 31, 2013

Sorry, I did forget to explain why I need this.

I'm not using the HTML generator. I have written a jade file which reads the form object and generates custom html. The reason for this is that I do not want to clutter the routes code with things like classes for css. Also, I did find it much easier to just render the form manualy than trying to tweek the output from the toHTML method.

ljharb added a note Oct 31, 2013

I'm hesitant to add support for use cases that aren't supported by this module. Specifically, you can certainly use forms purely for validation, but if you're also using it for rendering, the supported use case is to use only it for rendering.

Having forms work with templating systems, or providing a DOM tree, etc, would be a major feature and rewrite to do properly - and I'd prefer not to add such an important feature piecemeal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ljharb ljharb added a commit that referenced this pull request Mar 12, 2014
@ljharb ljharb Add number widget.
From #83.
ljharb commented Mar 12, 2014

I've added the widget. I don't want to expose getUserAttributes at this time. Thanks for the contribution!

@ljharb ljharb closed this Mar 12, 2014
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.