Merge pull request #2308 from ing-bank/fix/input-file-modelvalue

fix: add files more than once doesn't overwrite anymore
ing-bank · Jun 27, 2024 · 2015fa7 · 2015fa7
2 parents e4efb6d + 5917423
commit 2015fa7
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 15 deletions.
diff --git a/.changeset/rude-deers-taste.md b/.changeset/rude-deers-taste.md
@@ -0,0 +1,5 @@
+---
+'@lion/ui': patch
+---
+
+[input-file] add files more than once doesn't overwrite model value anymore
diff --git a/packages/ui/components/input-file/src/LionInputFile.js b/packages/ui/components/input-file/src/LionInputFile.js
@@ -176,6 +176,11 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField))
 
     /** @private */
     this.__duplicateFileNamesValidator = new DuplicateFileNames({ show: false });
+    /**
+     * @private
+     * @type {FileList | null}
+     */
+    this.__previouslyParsedFiles = null;
   }
 
   /**
@@ -283,8 +288,20 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField))
    * @returns {InputFile[]} parsedValue
    */
   parser() {
-    // @ts-ignore
-    return this._inputNode.files ? Array.from(this._inputNode.files) : [];
+    // parser is called twice for one user event; one for 'user-input-change', another for 'change'
+    if (this.__previouslyParsedFiles === this._inputNode.files) {
+      return this.modelValue;
+    }
+    this.__previouslyParsedFiles = this._inputNode.files;
+
+    const files = this._inputNode.files
+      ? /** @type {InputFile[]} */ (Array.from(this._inputNode.files))
+      : [];
+    if (this.multiple) {
+      return [...(this.modelValue ?? []), ...files];
+    }
+
+    return files;
   }
 
   /**
@@ -481,16 +498,17 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField))
     }
 
     this._inputNode.files = ev.dataTransfer.files;
-    // @ts-ignore
-    this.modelValue = Array.from(ev.dataTransfer.files);
-    // if same file is selected again, e.dataTransfer.files lists that file.
-    // So filter if the file already exists
-    // @ts-ignore
-    // const newFiles = this.__computeNewAddedFiles(Array.from(ev.dataTransfer.files));
-    // if (newFiles.length > 0) {
-    //   this._processFiles(newFiles);
-    // }
-    this._processFiles(Array.from(ev.dataTransfer.files));
+
+    if (this.multiple) {
+      const computedFiles = this.__computeNewAddedFiles(
+        /** @type {InputFile[]} */ (Array.from(ev.dataTransfer.files)),
+      );
+      this.modelValue = [...(this.modelValue ?? []), ...computedFiles];
+    } else {
+      this.modelValue = /** @type {InputFile[]} */ (Array.from(ev.dataTransfer.files));
+    }
+
+    this._processFiles(/** @type {InputFile[]} */ (Array.from(ev.dataTransfer.files)));
   }
 
   /**

diff --git a/packages/ui/components/input-file/test/lion-input-file.test.js b/packages/ui/components/input-file/test/lion-input-file.test.js
@@ -1,7 +1,7 @@
 import '@lion/ui/define/lion-input-file.js';
 import { Required } from '@lion/ui/form-core.js';
 import { getInputMembers } from '@lion/ui/input-test-helpers.js';
-import { expect, fixture as _fixture, html, oneEvent } from '@open-wc/testing';
+import { expect, fixture as _fixture, html, oneEvent, elementUpdated } from '@open-wc/testing';
 import sinon from 'sinon';
 
 /**
@@ -556,19 +556,22 @@ describe('lion-input-file', () => {
       });
       const fileListChangedEvent = await oneEvent(el, 'file-list-changed');
       filesListChanged(el, fileListChangedEvent);
-      await el.updateComplete;
+      await elementUpdated(el);
 
       // @ts-expect-error [allow-protected-in-test]
       expect(el._selectedFilesMetaData.length).to.equal(2);
+      expect(el.modelValue.length).to.equal(2);
 
       setTimeout(() => {
         mimicSelectFile(el, [file3, file4]);
       });
       const fileListChangedEvent1 = await oneEvent(el, 'file-list-changed');
       filesListChanged(el, fileListChangedEvent1);
-      await el.updateComplete;
+      await elementUpdated(el);
+
       // @ts-expect-error [allow-protected-in-test]
       expect(el._selectedFilesMetaData.length).to.equal(4);
+      expect(el.modelValue.length).to.equal(4);
     });
 
     it('should add multiple files and dispatch file-list-changed event ONLY with newly added file', async () => {
@@ -958,6 +961,37 @@ describe('lion-input-file', () => {
       expect(el.hasAttribute('is-dragging')).to.equal(false);
     });
 
+    it('should update modelValue on drop', async () => {
+      const list = new DataTransfer();
+      // @ts-ignore
+      list.items.add(file);
+      const droppedFiles = list.files;
+
+      // @ts-expect-error [allow-protected-in-test]
+      await el._processDroppedFiles({
+        // @ts-ignore
+        dataTransfer: { files: droppedFiles, items: [{ name: 'test.txt' }] },
+        preventDefault: () => {},
+      });
+      await el.updateComplete;
+
+      expect(el.modelValue.length).to.equal(1);
+
+      const list2 = new DataTransfer();
+      // @ts-ignore
+      list2.items.add(file2);
+
+      // @ts-expect-error [allow-protected-in-test]
+      await el._processDroppedFiles({
+        // @ts-ignore
+        dataTransfer: { files: list2.files, items: [{ name: 'test2.txt' }] },
+        preventDefault: () => {},
+      });
+      await el.updateComplete;
+
+      expect(el.modelValue.length).to.equal(2);
+    });
+
     it('should call _processFiles method', async () => {
       const list = new DataTransfer();
       // @ts-ignore