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

Can i validate HTML or other controls using Form-for #146

Open
akhare22sandeep opened this issue Jul 23, 2015 · 21 comments
Open

Can i validate HTML or other controls using Form-for #146

akhare22sandeep opened this issue Jul 23, 2015 · 21 comments

Comments

@akhare22sandeep
Copy link

Hi Brian,
Can I use Form-for module just for validating the my own HTML form without any of the Form-for control fields ? I really like the tool tip error messages.

@bvaughn
Copy link
Owner

bvaughn commented Jul 25, 2015

Hi @akhare22sandeep,

I'm not sure I understand your question. Are you saying that you want to make your own form HTML/markup but use the formFor library just for validation (without any of the formFor UI)? If so, then that really isn't something formFor supports, although another one of my libraries (like Forms JS) may be of interest to you.

If this isn't what you're asking, please give me an example or something to help me better understand. :)

@akhare22sandeep
Copy link
Author

Thanks for your replay Brian !
I have updated the question and you are right ! I am using Kendo UI controls and Pure html controls in my markup. Let me check Form JS. Do you have any demo link for Forms JS ?

@bvaughn
Copy link
Owner

bvaughn commented Jul 25, 2015

I see. formFor wasn't really created with that use-case in mind, although a friend recently mentioned something similar and I think it would be a neat idea to try to add support for in the next major release of formFor. I would need to finish my protractor testing effort first though so I could be confident I don't introduce any regressions.

You could probably use the Forms JS validators in your own project if you want. I wrote them to be pretty flexible. (I wasn't sure how many libraries we'd be using them with.) Unfortunately, Forms JS kind of stalled. I had initially hoped that the other contributors would have more time to contribute and when that wasn't happening, I decided that I didn't have the bandwidth to really take it on solo. It's unfortunate though because I think it got off to a strong start (and would have been the type of thing you are looking for).

But if you want to just use certain parts of the formFor UI (like errors and tooltips) you'd be better of forking and pulling out the directives/templates/styles. formFor was kind of written to be the whole solution vs being modular in that way.

@bvaughn
Copy link
Owner

bvaughn commented Aug 1, 2015

Going to close this issue for now as it's not a feature currently on the roadmap for formFor (mostly because I just don't have enough spare time to do that kind of refactoring).

If someone else would like to get involved and help- I think this would be a worthwhile goal and I'd be happy to support the effort.

@indrimuska
Copy link
Collaborator

What about a specific directive that uses transclusion?

Let's have a checkbox-field, its template is defined here. Maybe we can create a custom-field (with transclusion) that looks like this:

<custom-field>
   Some custom text
   <input
          some-custom-directive="true"
          aria-manager
          id="{{model.uid}}"
          name="{{attribute}}"
          type="checkbox"
          tabindex="{{tabIndex}}"
          ng-model="model.bindable"
          ng-checked="checked"
          ng-disabled="disable || model.disabled"
          ng-change="changed()">
</custom-field>

I don't know if it's already possible to do that, but I will try soon.

@bvaughn bvaughn reopened this Aug 3, 2015
@bvaughn
Copy link
Owner

bvaughn commented Aug 3, 2015

That's an interesting compromise idea, @indrimuska. I'll re-open this issue for now if you're interested in exploring that route. :)

@indrimuska
Copy link
Collaborator

Today I tried to create my first transcluded-field directive, starting from the guide in the Creating a custom field section.

The basic idea is very simple and easy to code, this is my plunker.

What do you think about it @bvaughn? I know, it's just a sketch, but I think it fits perfectly @akhare22sandeep's needs.

@bvaughn
Copy link
Owner

bvaughn commented Aug 6, 2015

Hi @indrimuska,

That's a pretty promising start! I like the direction.

I'm not sure I understand your comment in the scope about "every single option" being necessary. Could you elaborate?

@indrimuska
Copy link
Collaborator

I mean that (transcluded) scope is restricted only to those variables you set in the scope option of the directive, and you can't use any other attribute of any parent scope.

In the previous plunker I've set scope option like this:

scope: {
   attribute: '@',
   options: '=',
}

But what if I have a text input and I want to set a dynamic placeholder? Simply I can't do it, because placeholder is not in the scope (check out this plunker and try to uncomment script.js at line 28).

I don't know what is the best solution for this use case, I think that we have to set every single attribute we want to support into the scope option.

We could have an optional "jolly" attribute to pass a custom object:

scope: {
   // all supported attributes first
   attribute: '@',
   placeholder: '@?',
   // ...
   // the "jolly" one :)
   custom: '=?'
}

So that we can use it into the transcluded scope (plunker):

<div ng-init="custom={description: 'my field description' }">
   <transcluded-field attribute="name" custom="custom">
      <input placeholder="{{placeholder}}">
      <p>{{custom.description}}</p>
   </transcluded-field>
</div>

@bvaughn
Copy link
Owner

bvaughn commented Aug 8, 2015

What if you got rid of the isolate scope and transclusion entirely?

  .directive('altField', ['FieldHelper', function (FieldHelper) {
    return {
      restrict: 'E',
      require: '^formFor',
      transclude: true,
      template: '<span ng-transclude></span>',
      link: function ($scope, $element, $attributes, formForController, transclude) {
        // FieldHelper expects an 'attribute' property on $scope
        $scope.attribute = $attributes.attribute;
        FieldHelper.manageFieldRegistration($scope, $attributes, formForController);
      }
    };

Here's an updated Plunker.

@indrimuska
Copy link
Collaborator

Really beautiful and clean solution, but it has a little problem with the naming system.
Doing this, you allow the user to override attributes required to perform a correct behavior of the plugin, the simplest example is using a controller named model with an attribute named bindable:

<html ng-app="myApp" ng-controller="Controller as model">
.controller('Controller', ['$scope', function ($scope) {
   this.bindable = 'hello world';
})

See this plunker. As expected, nothing works fine if someone uses these "special" names, not even validation on submit.

@bvaughn
Copy link
Owner

bvaughn commented Aug 11, 2015

Hmm. That's fair, although I think it's probably pretty unlikely to occur.

That being said we could address this by refactoring to use a more "private" naming convention (e.g. $model.bindable). I'm not convinced that's necessary or worth it. (It would require anyone with custom fields or other overrides to make changes.)

I'm tempted to say that we could just avoid this unlikely case with clear documentation. What are your thoughts?

We could even check for the presence of such a $scope property and throw an error (or console.warn).

@bvaughn bvaughn closed this as completed Aug 11, 2015
@bvaughn
Copy link
Owner

bvaughn commented Aug 11, 2015

Wow. I totally clicked "close and comment" without meaning to. Sorry.

@bvaughn bvaughn reopened this Aug 11, 2015
@indrimuska
Copy link
Collaborator

I don't know, the more I try to use this solution, the more I found some little issues.
Now the problem is the loss of two-way binding between model.bindable and ctrl.myInput (data inside the main controller scope).

The line that you have inserted - $scope.attribute = $attributes.attribute, required by FieldHelper - allows the correct behavior of the plugin just for internal purposes, I mean just for validation and error reporting. But it doesn't bind to the original field, so that any change to model.bindable doesn't reflect the data it represent (maybe we could perform 2-way binding manually).

Check this plunker.

@bvaughn
Copy link
Owner

bvaughn commented Aug 14, 2015

I'm not sure I see what you're describing. I updated my original plunker (see here) to also log the form-data value on submit and it's got an updated value like I'd expect.

Edit: Your Plunker had an error in it. You were referencing ctrl.myInput when you should have been referencing ctrl.data.myInput. Check out the updated Plunker here: http://plnkr.co/edit/EBtKwbzXHnUZJqQTyutN?p=preview

@indrimuska
Copy link
Collaborator

Oh you're right, the problem was mine, your plunker seems to work well.

Just another question: does your solution works even if there are 2 (or more) transcluded-field inside the same form? Since there is no longer a private scope for the directive, all fields share the same scope, so that if one of them updates an attribute (e.g. model.bindable) it reflects to every other transcluded-field that makes use of attributes with this same name (model.bindable inside the other field).

Check out this updated punker.

@bvaughn
Copy link
Owner

bvaughn commented Aug 16, 2015

Hah! Good point!

I'm at a bit of a loss at the moment. Tried a couple of things in a local copy but without any luck.

@indrimuska
Copy link
Collaborator

I'm still here, I haven't forgot this issue. 😄

I am currently using a solution with 5 attributes, one of which is the custom attribute that I already wrote (the "jolly").

That's the behavior of my personal transcluded-field:
http://plnkr.co/edit/CtW1IWHSH3YD96gDaFQK?p=preview

I think that there's no need to add all possible attributes in the transcluded scope, just because, for example, if I have a <select> I don't necessarily have to use the attribute options (with this name), and so for all kind of input. Users can update their view, so they can use other naming system or other objects. The only attributes they need are the ones for validation, that's why I added a <field-error> element in my template.


EDIT: this version makes use of an HTML placeholder (<span class="placeholder"></span>) to let user choose where to put the transcluded content. The reason why I haven't used a simple ng-transclude directive si that it creates a new isolated scope, so that I can't bind the parent attributes.

@bvaughn
Copy link
Owner

bvaughn commented Sep 5, 2015

Hi again @indrimuska :)

Your approach seems reasonable. Would you like to create a PR for me to take a look at?

@indrimuska
Copy link
Collaborator

Yes, I could try, but please be generous with me, I coded with typescript just one time! 😄

@bvaughn
Copy link
Owner

bvaughn commented Sep 5, 2015

Of course :)

On Saturday, September 5, 2015, indrimuska notifications@github.com wrote:

Yes, I could try, but please be generous with me, I coded with typescript
just one time! [image: 😄]


Reply to this email directly or view it on GitHub
#146 (comment)
.

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

No branches or pull requests

3 participants