Skip to content
This repository has been archived by the owner on May 5, 2018. It is now read-only.

Initial version of the ui-input directive - RFC #191

Closed
wants to merge 3 commits into from

Conversation

pkozlowski-opensource
Copy link
Member

This is initial version of the code for the new ui-input family of directives (ui-input, ui-select, ui-textarea) as described in the issue #189.

You can see it in action in this plunk: http://embed.plnkr.co/TT1t1p

From the last discussion on the mailing list several things have changed:

  • validation messages now can be configured on a module level
  • familly and kind of a input's template can be configured on a module level
  • select and textarea are supported

There are still plenty of things that should be discussed done (no tests for the moment!) but I feel like this is a good starting point. I've started to use it a bit already and it is extremely powerful, it really makes it possible to write forms with the consistent look & fell - fast!

Let's discuss the concepts behind and the implementation itself!

@petebacondarwin
Copy link
Member

Nice one Pawel. Does it really need the InputHelper as a service rather than simply a local function?

@ProLoser
Copy link
Member

ProLoser commented Sep 8, 2012

@petebacondarwin He's following an example I did in keypress so that I could reuse the same linking function in multiple directives. ATM I don't know a better way to do this

@pkozlowski-opensource

  • uiConfig.uiinput should just be uiConfig.input
  • Perhaps uiConfig.input.validation should just be uiConfig.validation? In case we break this down into smaller pieces and want to reuse the validation? Besides, people would probably want to reuse wherever they stored these rules for the rest of their app when ui-input isn't applicable.
  • Is there no way / thought yet on letting the user define a template block that we use? I think this is the best approach so far. The user could either specify an ID of the block or we would find some block with the default name (like ui-input). We could pass meta data such as $input so that people can check what type of input it is and execute a switch, etc.
  • After re-reading the code I realized you declared 3 separate directives. Perhaps we should just do 1 directive and add 'select' and 'textarea' to the type attribute? Rather than parsing the DOM element however, you could just pass a string to the factory to specify what mode it should behave in. Although in this configuration I can't see how.

@pkozlowski-opensource
Copy link
Member Author

@petebacondarwin I would love to avoid declaring a service, but simply don't know how to do it.... We have discussed this with @ProLoser and couldn't find any better approach... But it would be great if we could avoid declaring those helper services....

@ProLoser still going over your comments but it is true that we could pull validation to the uiConfig.validation. This way it could be re-used with uiValidate as well.

@ProLoser
Copy link
Member

ProLoser commented Sep 8, 2012

Another bullet

  • Should this really be an attribute only? I sort of envisioned it as its own element so that we could switch out what it actually becomes
  • Do we really need ui-select and ui-textarea if this can be attached to an element of that type? For this reason alone we should probably remove those varieties (and thereby remove the factory too). ui-input would just be applicable to those 3 elements
  • Perhaps the behavior should be different depending on if it's an attribute vs an element. If it's an attribute, it could merely decorate the input with additional information (such as an ID and name) but if it's a <ui-input> then it could potentially spit out the and any other DOM formatting the template provides too. Perhaps in element form it would use the template and otherwise just decorate?
  • If we split this up into a 2 step approach, perhaps this would be a good way to define a more modular directive. For instance having a template outputter directive () and a form input decorator () with different names of course :)
  • Possible attributes of the $input object available in the template block:
    • id (generated?)
    • name (generated?)
    • error (reference to `form.name.$error)
    • type (could be reused even if the element attached to was a textarea or select)

@ProLoser
Copy link
Member

Since I will be needing this in the very near future, I'm working on my interpretation of an alternative structure: http://plnkr.co/edit/Ehnvjl Unfortunately, unlike @pkozlowski-opensource mine doesn't work at the moment lol.

Major differences

  • uiConfig.uiinput changed to uiConfig.input
  • 'family' and 'kind' dropped in exchange for a 'template' attribute which will be used to specify a url OR template ID
  • scope.new() dropped for scope: true
  • require: ?ngModel instead of element.controller('ngModel')
  • ui-select and ui-textarea merged into ui-input (1 directive instead of 3). This will allow complete flexibility for cases such as checkboxes and radio too
  • $input instead of $control now holds all meta-data (the name 'control' is derived from bootstrap, while input is the plugin name)
  • $field and $input (previously $control) are merged together into one object
  • ng-transclude is used instead of partial.find('input') for transclusion. It will be supported on multiple elements in the template
  • input-label changed to simply label
  • ui-input should be replaced by new template, not nested into

Certain thoughts / ideas

  • Search the DOM for a template by ID, if none is found try pinging the URL for a partial?
  • Add a ui-form directive to allow you to set common defaults to a batch of inputs (such as the template url)? If we create this second directive, perhaps it should have it's own controller which can then be injected into ui-input?
  • Is it worth it to bother with uiConfig.input.validation? Couldn't the dev just do $rootScope.validation and reuse that?
  • ng-options="items" for checkboxes and radios to access too?
  • Human-readable IDs based on ng-model? (this is done in CakePHP and very convenient)

For some reason $compile() is throwing an error
Stubbed out uiForm dependency but need help creating the directive
@ProLoser
Copy link
Member

ProLoser commented Nov 1, 2012

If you guys could help me figure out why $compile() is throwing a completely cryptic error that would be awesome.


//expose model to a field's template
scope.$input = input;
$compile(element.contents())(scope);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think $compile errors because of the ng-transcludes. It doesn't know what to do. You'll have to manually transclude the elements with ng-transclude attribute, then remove them before $compiling.

OR alternatively, you could try putting in all the html during compile phase, and it might know how to use the transcludes. But you don't scope during compile phase, so that would hard to do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Touché, I guess I just figured that ng-transclude was not actually a
directive but a subset.
On Oct 31, 2012 7:46 PM, "Andy Joslin" [email protected] wrote:

In modules/directives/input/input.js:

  •      element.html(response);
    
  •      var inputEl = element.find('[ng-transclude]');
    
  •      angular.forEach(tAttrs, function (value, key) {
    
  •        if (key.charAt(0) !== '$' && ['src','uiInput'].indexOf(key) === -1) {
    
  •          inputEl.attr(snake_case(key, '-'), value);
    
  •        }
    
  •      });
    
  •      //prepare validation messages
    
  •      input.validation = angular.extend({}, uiConfig.input.validation, scope.$eval(tAttrs.validation));
    
  •      //expose model to a field's template
    
  •      scope.$input = input;
    
  •      $compile(element.contents())(scope);
    

I think $compile errors because of the ng-transcludes. It doesn't know
what to do. You'll have to manually transclude the elements with
ng-transclude attribute, then remove it before $compiling.


Reply to this email directly or view it on GitHubhttps://github.com//pull/191/files#r2001035.

@ProLoser
Copy link
Member

ProLoser commented Nov 1, 2012

So things I'm running into:

  • You will usually end up doing ng-model=$parent.whatever (no big deal)
  • How do I access the model from somewhere OTHER than the ng-include'd element? (Like for rendering)
  • All properties are being transfered to the ng-include AND the $input for easy flexibility / extendability
  • How do we do radios/checkbox options?
    • Someone could probably do <ui-input values="myOptions"> and <input ng-repeat="{{$input.values}}"> or something?
    • What if you want to repeat on <li> or some other element?
    • No matter what, we should not hard-code this solution
  • Need to add uiForm defaults
  • Need to make sure which items override on $input is well thought out

I added a template at the top of the file that I'm using for testing.

@tsenying
Copy link

Does someone have a working example of the current code?
The example at the top of the input.js file works, but fails if I add the type attribute to the directive at the line:

inputEl.attr(snake_case(key, '-'), value);

because jQuery throws the error: "type property can't be changed"
apparently due to an IE problem with changing the element type: http://stackoverflow.com/questions/1544317/jquery-change-type-of-input-field

I would like to get the ui-input directives with different input types in the plunkers previously posted here by pkozlowski-opensource and ProLoser to work (they don't with the current code)

A plunker showing the failure of the directive with type attribute is here: http://plnkr.co/edit/8jN2PA
I had to include jQuery so that the line below would work:

var inputEl = element.find('[ng-transclude]');

Also, not understanding why the $parent scope is used in the ui-input directive ng-model attribute?

<ui-input src="'input-horizontal.html'" ng-model="model.required" type="text" required label="Required input"></ui-input>

@ProLoser ProLoser mentioned this pull request Jan 26, 2013
@petebacondarwin
Copy link
Member

Here is an update on this: http://plnkr.co/edit/3zMsNnpNfOFwExSqLj2I?p=preview.
It is a WIP but it "works" and I think it provides a very clean API.
I would appreciate any suggestions or comments.

@pkozlowski-opensource
Copy link
Member Author

I guess this one is no longer relevant...

@ProLoser
Copy link
Member

I'd still like to see this implemented eventually but yeah who's got the time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants