danialfarid / ng-file-upload

Lightweight Angular directive to upload files with optional FileAPI shim for cross browser support
MIT License
7.87k stars 1.59k forks source link

Issue with ngf-max-files #1377

Closed madurapa closed 8 years ago

madurapa commented 8 years ago

Here is the scenario:

I have set properties as below, ngf-model-invalid="invalidFiles" ng-model="files" ngf-multiple="true" ngf-keep="'distinct'" ngf-max-size="5MB" ngf-max-files="10"

I have set ngf-max-files="10" with ngf-multiple="true" and select more than 10 files at once then all files were assigned to ngf-model-invalid="invalidFiles" then I removed one by one till the count gets 10 but it didn't reassign to ng-model="files"

Here is the screenshot to make it more clear: error-max-files

danialfarid commented 8 years ago

You need to set them back to ng-model. ng-model-invalid is not watched so the changes won't update validations.

madurapa commented 8 years ago

Isn't it something that the directive should be taking care of? It would be better if you could do this as an enhancement in future version. It's kinda pointless to write much code after use an efficient directive like this.

danialfarid commented 8 years ago

The thing is that in angular each element is bound to the ng-model scope variable and that's a two way binding, So any changes in the ng-model variable would trigger the model update and validation. Also in angular when you have validation only the valid inputs would be allowed to be set unless you have ng-model-options allowInvalid. So the behaviour here is what angular normally do, the ngf-model-invalid is just a helper directive to just show the invalid selected files and it is not part of the model.

The other option is to make the ng-model to be and object containing both valid and invalid files like {valid: validFiles, invalid: invalidFiles} but then that is a bit counter intuitive and this approach or listening to ngf-model-invalid adds extra complication and many corner cases.

So if you want to change the file's array and have validations updated just have the allowInvalid model option set and use the ng-model to have your validations updated.

madurapa commented 8 years ago

Well I have ended up with bunch of lines code here (maybe not the perfect way)

HTML

<div class="form-group" ng-class="{ 'has-error': newMessage.files.$dirty && newMessage.files.$invalid }">
   <div class="col-sm-12 controls">
      <div class="btn vd_btn vd_bg-grey"
         ngf-pattern="'.pdf,.doc,.docx,.xls,.xlsx,.txt,.rtf,.jpg,.jpeg,.png,.gif'"
         ngf-accept="'.pdf,.doc,.docx,.xls,.xlsx,.txt,.rtf,.jpg,.jpeg,.png,.gif'"
         name="files" ng-model="files"
         ngf-multiple="true" ngf-keep="'distinct'" ngf-max-size="15MB"
         ngf-max-total-size="5MB"
         ngf-max-files="10" ng-model-options="{allowInvalid: true}"
         ngf-select="fileSelect()">
         <i class="fa fa-files-o"></i> &nbsp;
         Attach Files
      </div>
      <div class="mgtp-15" ng-show="files.length > 0">
         <ul>
            <div ng-repeat="file in files track by $index">
               <li data-ng-class="{'vd_red': file.$error}">
                  {{file.name}}
                  <a href="" ng-click="removeAttachment($index)" class="vd_red">
                  <i class="fa fa-remove"></i>
                  </a>
               </li>
            </div>
         </ul>
      </div>
      <div class="help-block" ng-messages="newMessage.files.$error"
         ng-if="newMessage.files.$dirty && newMessage.files.$invalid">
         <span ng-message="pattern" class="pull-left">You can only upload following files: pdf, doc, docx, xls, xlsx, txt, rtf, jpg, jpeg, png, gif</span>
         <span ng-message="maxSize"
            class="pull-left">Your file(s) larger than the maximum allowed of 5MB per a file.</span>
         <span ng-message="maxFiles" class="pull-left">You can only select maximum 10 files at a time.</span>
         <span ng-message="maxTotalSize" class="pull-left">The total size of attached files exceeds than 20MB</span>
      </div>
   </div>
</div>

JS

$scope.checkFiles = checkFiles;
$scope.fileSelect = fileSelect;
$scope.removeAttachment = removeAttachment;

$scope.messageForm = [];
var messageArr = {};

function fileSelect() {
    $scope.checkFiles();
}

function removeAttachment(key) {
    $scope.files.splice(key, 1);
    $scope.checkFiles();
}

function checkFiles() {
    var files = $scope.files;
    var totalSize = 0;
    var maxFiles = 10;
    var maxSize = 5;
    var maxTotalSize = 20;
    var flagMaxTotalSize = false;
    var flagMaxSize = false;
    var flagPattern = false;
    var pattern = ['pdf', 'doc', 'docx', 'xls', 'xlsx', 'txt', 'rtf', 'jpg', 'jpeg', 'png', 'gif'];

    if (files.length > maxFiles) {

        for (var i = 0; i < files.length; i++) {
            $scope.files[i]['$error'] = 'maxSize';
        }

        $scope.newMessage.files.$setValidity("maxFiles", false);

    } else if (files.length > 0 && files.length <= maxFiles) {

        $scope.newMessage.files.$setValidity("maxFiles", true);

        _.each(files, function(file) {
            totalSize = totalSize + file.size;
        });

        if ((totalSize / (1024 * 1024)).toFixed(2) > maxTotalSize) {
            flagMaxTotalSize = true;
        }

        for (var i = 0; i < files.length; i++) {
            if (flagMaxTotalSize) {
                $scope.files[i]['$error'] = 'maxTotalSize';
            } else if (($scope.files[i].size / (1024 * 1024)).toFixed(2) > maxSize) {
                flagMaxSize = true;
                $scope.files[i]['$error'] = 'maxSize';
            } else if (pattern.indexOf($scope.files[i].name.slice(($scope.files[i].name.lastIndexOf(".") - 1 >>> 0) + 2)) === -1) {
                flagPattern = true;
                $scope.files[i]['$error'] = 'pattern';
            } else {
                delete $scope.files[i].$error;
                delete $scope.files[i].$errorMessages;
                delete $scope.files[i].$errorParam;
            }
        }

        if (flagMaxTotalSize) {
            $scope.newMessage.files.$setValidity("maxTotalSize", false);
        } else if (flagMaxSize) {
            $scope.newMessage.files.$setValidity("maxSize", false);
        } else if (flagPattern) {
            $scope.newMessage.files.$setValidity("pattern", false);
        } else {
            $scope.newMessage.files.$setValidity("maxTotalSize", true);
            $scope.newMessage.files.$setValidity("maxSize", true);
            $scope.newMessage.files.$setValidity("pattern", true);
        }

    } else {
        $scope.newMessage.files.$setValidity("maxTotalSize", true);
        $scope.newMessage.files.$setValidity("maxSize", true);
        $scope.newMessage.files.$setValidity("pattern", true);
        $scope.newMessage.files.$setValidity("maxFiles", true);
    }
}

Other thing I have noticed Eg: Lets say ngf-pattern="'.pdf'" AND ngf-max-total-size="5MB" AND ngf-max-files="2"

File error array will be like below which is not quite correct

    {
      "$error": "maxSize",
      "$errorMessages": {
        "name": true
      },
      "$errorParam": 2
    },
    {
      "$error": "maxSize",
      "$errorMessages": {
        "name": true
      },
      "$errorParam": 2
    },
    {
      "$error": "maxSize",
      "$errorMessages": {
        "name": true
      },
      "$errorParam": 2
    }
danialfarid commented 8 years ago

1) what requirement? 2) $errorParam will contain the restriction value of the validation. 3) Could you create a jsfiddle with files that you are using so I could reproduce.

danialfarid commented 8 years ago

I will close this until steps to reproduce is provided.

madurapa commented 8 years ago

I forgot to mention before these issues came when ngf-keep="'distinct'" AND ngf-multiple="true" set.

When you select one by one file it is working fine but when you are selecting multiple files at once this issues came.

Here is the jsfiddle https://jsfiddle.net/rapa/q7hq7pze/2/

To reproduce this select more than five files at once then look at the $errorParam

Requirements are

Let me know if you need anything from my side.

danialfarid commented 8 years ago

It is fixed at 12.0.3. For the last one use file.$errorParam, like the demo page which is showing the validation values when there is an error.

madurapa commented 8 years ago

Thanks for the fixes mate!

Still there are some issues

https://jsfiddle.net/rapa/q7hq7pze/5/

danialfarid commented 8 years ago

Can you specify the exact steps to reproduce? It is working fine for me.

madurapa commented 8 years ago

Use the jsfiddle link I have created then select one file more than 1MB, couple of invalid pattern files, select couple of valid files, total file count should be more than 6.

danialfarid commented 8 years ago

then what? provide steps like 1 2 3 and what is expected and what is actual. That's the steps to reproduce

madurapa commented 8 years ago

edit fiddle - jsfiddle 2016-03-16 13-00-20

Is this make sense?

danialfarid commented 8 years ago

It won't necessarily run all the validations when there is an error for the file. When the file is invalid there is no need to run all other validations on it since it would still be invalid. That's why you don't see other validation errors on the already invalid files.

madurapa commented 8 years ago

But it would be really needful if anyone wants to show what are the errors with each files in the view for better UX

danialfarid commented 8 years ago

I don't agree with that, let's say the user chooses 1000 files and only 5 is allowed you don't want to run all the validations on the extra files and consume memory cpu etc for files that are not allowed.

madurapa commented 8 years ago

Agreed!

bkhushang commented 7 years ago

I have found somewhat same issue. Please help me.

Max files = 10 files File Max size = 2MB.

I have upload 13 images at once and from them 5th image is of 3 MB. So ngf-max-size removes that file from upload array and put it into error files and it is only taking first 10 files only but here 5th image file is removed so it should take 11th image file as this file is fulfill the requirement to upload. Is there any way to solve this?

You can see the screen shot of my scenario. screenshot from 2017-01-11 16 06 37

danialfarid commented 7 years ago

You can have ngf-ignore-invalid="'maxSize'" and then handle the maxSize error manually on each file.