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

fix: add files more than once doesn't overwrite anymore #2324

Merged
merged 4 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/dry-turkeys-flash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[input-file] add files more than once doesn't overwrite model value anymore
42 changes: 30 additions & 12 deletions packages/ui/components/input-file/src/LionInputFile.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField))

/** @private */
this.__duplicateFileNamesValidator = new DuplicateFileNames({ show: false });
/**
* @private
* @type {FileList | null}
*/
this.__previouslyParsedFiles = null;
}

/**
Expand Down Expand Up @@ -282,8 +287,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;
}

/**
Expand Down Expand Up @@ -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)));
}

/**
Expand Down
40 changes: 37 additions & 3 deletions packages/ui/components/input-file/test/lion-input-file.test.js
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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
Expand Down