search

formly-js / angular-formly

JavaScript powered forms for AngularJS
http://docs.angular-formly.com
MIT License
2.23k stars 405 forks source link

VM loses form when form is inside an ng-if #633

Closed chilversc closed 8 years ago

chilversc commented 8 years ago

Steps to reproduce

  1. Create a form with validation inside an ng-if block.
  2. Bind <formly-form form="vm.form">
  3. Toggle the state of the ng-if to hide the form.
  4. Toggle the state of the ng-if to show the form.

    Expected result

The form should contain all its fields and show as invalid.

Actual result

vm.form continues to refer to the old form that is now blank..

Demo

View plnkr

<!DOCTYPE html>
<html>
  <head>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/api-check/7.5.5/api-check.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.4.7/angular.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/angular-formly/7.3.0/formly.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/angular-formly-templates-bootstrap/6.1.5/angular-formly-templates-bootstrap.min.js"></script>
    <script>
      angular.module('plnkr', ['formly', 'formlyBootstrap'])
        .controller('mainController', function($scope) {
          this.model = {};
          this.fields = [{
             key: 'name',
             type: 'input',
             templateOptions: {
               type: 'text',
               label: 'Name',
               required: true
             }
            }];
          this.busy = false;
        });      
    </script>
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.3.5/css/bootstrap.min.css" />
  </head>

  <body ng-app="plnkr">
    <div class="container" ng-controller="mainController as vm">
      <h1>Test</h1>

      <h1>Steps to reproduce</h1>
      <ol>
        <li>Leave the name field blank</li>
        <li>Check the busy flag to hide the form</li>
        <li>Un-check the busy flag to display the form</li>
      </ol>

      <h1>Expected result</h1>
      <p>The form should contain all its fields and show as invalid</p>

      <h1>Actual result</h1>
      <p>vm.form continues to refer to the old form that is now blank.</p>

      <h1>Form</h1>
      <label><input type=checkbox ng-model=vm.busy> Busy</label>

      <div ng-if="!vm.busy">
        <formly-form form="vm.form" model="vm.model" fields="vm.fields"></formly-form>
      </div>
      <div ng-if="vm.busy">
        Working...
      </div>

            <span class="label label-success" ng-show="vm.form.$valid">Valid</span>
            <span class="label label-danger" ng-show="vm.form.$invalid">Invalid</span>

      <pre>{{vm.form|json}}</pre>
    </div>
  </body>

</html>
sukrosono commented 8 years ago

hey i try view the plnkr and it just work perfectly :smile:

chilversc commented 8 years ago

You sure? You have to toggle busy on and off again. You'll note how in the after, although the form is visible the form does not contain any of the form fields and is listed as being valid despite the required field being blank.

Before

"Invalid"

{
  "$error": {
    "required": [
      {
        "$validators": {},
        "$asyncValidators": {},
        "$parsers": [],
        "$formatters": [
          null
        ],
        "$viewChangeListeners": [],
        "$untouched": true,
        "$touched": false,
        "$pristine": true,
        "$dirty": false,
        "$valid": false,
        "$invalid": true,
        "$error": {
          "required": true
        },
        "$name": "formly_1_input_name_0",
        "$options": null
      }
    ]
  },
  "$name": "formly_1",
  "$dirty": false,
  "$pristine": true,
  "$valid": false,
  "$invalid": true,
  "$submitted": false,
  "formly_1_input_name_0": {
    "$validators": {},
    "$asyncValidators": {},
    "$parsers": [],
    "$formatters": [
      null
    ],
    "$viewChangeListeners": [],
    "$untouched": true,
    "$touched": false,
    "$pristine": true,
    "$dirty": false,
    "$valid": false,
    "$invalid": true,
    "$error": {
      "required": true
    },
    "$name": "formly_1_input_name_0",
    "$options": null
  }
}

After

"Valid"

{
  "$error": {},
  "$name": "formly_1",
  "$dirty": false,
  "$pristine": true,
  "$valid": true,
  "$invalid": false,
  "$submitted": false
}
chilversc commented 8 years ago

This does not occur if you use:

<form name="vm.form">
    <formly-form model="vm.model" fields="vm.fields"></formly-form>
</form>

So perhaps this should be a documented limitation if the goal is to move away from using <formly-form> without an outer <form> element.

Note that the problem still exists if you use <formly-form form="vm.form" root-el="form">.

chilversc commented 8 years ago

Although, it could still be a problem even when using <form> if you want to reference different sections of a formly form. http://embed.plnkr.co/tmoPJtWvVOZ3eK0XbzHu/

chilversc commented 8 years ago

I can make this work with the current form by assigning both the form and the formly-form to the same property:

<form name="vm.form">
    <formly-form form="vm.form" model="vm.model" fields="vm.fields"></formly-form>
</form>

Although it works, something about it just feels wrong. Though it does now update the form variable correctly and works with the standard method of setting $submitted to show field errors.

http://embed.plnkr.co/ycI1HbkLRR1o8i3CNzmW/

kentcdodds commented 8 years ago

I can make this work with the current form by assigning both the form and the formly-form to the same property

This is how it's intended to work. You're basically passing the form to formly-form telling it what to use for the NgFormCtrl. Formly does some magic under the hood to manage the NgFormCtrl to make the API more clear. Unfortunately that leads to issues like the one you're seeing, but most of the time it's better than the alternative.

I don't think there's anything I can do to fix this. Just name your form and pass it to formly-form.