From 74940911a0d3ccc806d67c638d927f2dd3fd75f5 Mon Sep 17 00:00:00 2001 From: ByoungYong Kim Date: Tue, 25 Jun 2024 16:22:56 +0200 Subject: [PATCH 1/4] fix: add files more than once doesn't overwrite anymore --- .../input-file/src/LionInputFile.js | 40 ++++++++++++++----- .../input-file/test/lion-input-file.test.js | 40 +++++++++++++++++-- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/packages/ui/components/input-file/src/LionInputFile.js b/packages/ui/components/input-file/src/LionInputFile.js index 094565c98..d12ad032d 100644 --- 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,15 +498,17 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) } this._inputNode.files = ev.dataTransfer.files; + + const computedFiles = this.__computeNewAddedFiles( + /** @type {InputFile[]} */ (Array.from(ev.dataTransfer.files)), + ); + if (this.multiple) { + this.modelValue = [...(this.modelValue ?? []), ...computedFiles]; + } else { + this.modelValue = computedFiles; + } + // @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)); } @@ -609,6 +628,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) .map(({ systemFile }) => /** @type {InputFile} */ (systemFile)); if (_successFiles.length > 0) { + // this.modelValue = [...this.modelValue, _successFiles]; this._dispatchFileListChangeEvent(_successFiles); } } 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 index 85750eb5f..df576e633 100644 --- 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 From 58d56b2b88beab516c1eb32ae4fc9d5045f1263e Mon Sep 17 00:00:00 2001 From: ByoungYong Kim Date: Thu, 27 Jun 2024 09:12:25 +0200 Subject: [PATCH 2/4] doc: add changeset --- .changeset/rude-deers-taste.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rude-deers-taste.md diff --git a/.changeset/rude-deers-taste.md b/.changeset/rude-deers-taste.md new file mode 100644 index 000000000..e11c0f9ff --- /dev/null +++ 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 From d75976a23893297cf8cbd5f04de6794a006476a9 Mon Sep 17 00:00:00 2001 From: ByoungYong Kim Date: Thu, 27 Jun 2024 10:18:38 +0200 Subject: [PATCH 3/4] fix: Fix a bug when the same file is added on the single file input, the model value gets empty --- packages/ui/components/input-file/src/LionInputFile.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ui/components/input-file/src/LionInputFile.js b/packages/ui/components/input-file/src/LionInputFile.js index d12ad032d..f9f67c08e 100644 --- a/packages/ui/components/input-file/src/LionInputFile.js +++ b/packages/ui/components/input-file/src/LionInputFile.js @@ -499,13 +499,13 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) this._inputNode.files = ev.dataTransfer.files; - const computedFiles = this.__computeNewAddedFiles( - /** @type {InputFile[]} */ (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 = computedFiles; + this.modelValue = Array.from(ev.dataTransfer.files); } // @ts-ignore From 5917423bc9d14164fe7ee98e808a4ad5d24ca226 Mon Sep 17 00:00:00 2001 From: ByoungYong Kim Date: Thu, 27 Jun 2024 11:37:52 +0200 Subject: [PATCH 4/4] Fix a lint error, remove unnecessary lines --- packages/ui/components/input-file/src/LionInputFile.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/ui/components/input-file/src/LionInputFile.js b/packages/ui/components/input-file/src/LionInputFile.js index f9f67c08e..b17a602b7 100644 --- a/packages/ui/components/input-file/src/LionInputFile.js +++ b/packages/ui/components/input-file/src/LionInputFile.js @@ -505,11 +505,10 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) ); this.modelValue = [...(this.modelValue ?? []), ...computedFiles]; } else { - this.modelValue = Array.from(ev.dataTransfer.files); + this.modelValue = /** @type {InputFile[]} */ (Array.from(ev.dataTransfer.files)); } - // @ts-ignore - this._processFiles(Array.from(ev.dataTransfer.files)); + this._processFiles(/** @type {InputFile[]} */ (Array.from(ev.dataTransfer.files))); } /** @@ -628,7 +627,6 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) .map(({ systemFile }) => /** @type {InputFile} */ (systemFile)); if (_successFiles.length > 0) { - // this.modelValue = [...this.modelValue, _successFiles]; this._dispatchFileListChangeEvent(_successFiles); } }