From 8b9c8050e53a949451afe3958bb252425ba55b9b Mon Sep 17 00:00:00 2001 From: Danny Moerkerke Date: Wed, 7 Jun 2023 12:57:34 +0200 Subject: [PATCH] Fix/upload response (#2000) * feat(input-file): create input-file component * chore: improvements after review * chore: update after review * chore: update translations * chore: - fixed demo with form submit, submit was not prevented - fixed checking allowed file extensions - fixed clicking on select file button in drag and drop area * chore: since the input-file does not upload files itself but enables user to select files, I replaced "upload" and "upload" with "select" and "selected" where applicable * chore: - removed unused properties allowedFileTypes and allowedFileExtensions from lion-input-file - cleaned up docs * chore: - changed type Array. to Array - removed redundant type definition * fix: - FocusMixin: moved registering events for from connectedCallback to firstUpdated since _focusableNode is sometimes not available yet - SlotMixin: changed updated to update in since slots were rendered too late (related to previous fix in FocusMixin.js) * fix: renamed lion-uploaded-file-list.js to lion-selected-file-list.js * fix: fixed test for lion-selected-file-list * fix: fixed typ * wip * fix: - fixed issue with multiple file selection where element would not select valid files after invalid ones - added getMessage method to FileValidation that returns empty string to prevent message being shown that error message must be configured - fixed tests * chore: replaced `uploadOnFormSubmit` with `uploadOnSelect` and flipped the default value to false. When `uploadOnSelect` is set to true, the file will be uploaded as soon as it is selected. * fix: - replaced `uploadOnFormSubmit` with `uploadOnSelect` and flipped the default value to false. When `uploadOnSelect` is set to true, the file will be uploaded as soon as it is selected. - fixed issue where a valid file was not selected and added to the file list if it was preceded by an invalid file * chore: removed redundant README.md * fix: fixed failing test * chore: added missing type annotation * chore: annotated event param as optional * chore: corrected property names in demo and removed demo of prefilled state * chore: renamed `_fileSelectResponse` back to `uploadResponse` and restored demo of prefilled state --------- Co-authored-by: gvangeest Co-authored-by: Thijs Louisse --- docs/components/input-file/use-cases.md | 11 ++- .../input-file/src/LionInputFile.js | 36 ++++----- .../input-file/src/LionSelectedFileList.js | 2 +- .../input-file/test/lion-input-file.test.js | 79 +++++++++---------- .../test/lion-selected-file-list.test.js | 2 +- .../input-file/types/input-file.d.ts | 6 +- 6 files changed, 66 insertions(+), 70 deletions(-) diff --git a/docs/components/input-file/use-cases.md b/docs/components/input-file/use-cases.md index 5692bfd47..35d7c5f04 100644 --- a/docs/components/input-file/use-cases.md +++ b/docs/components/input-file/use-cases.md @@ -26,8 +26,7 @@ API calls to upload the selected files can be done in below two ways which is dr help-text="Signature scan file" max-file-size="1024000" accept="application/PDF" - upload-on-form-submit - .uploadResponse="${uploadResponse}" + upload-on-select > ``` @@ -316,7 +315,7 @@ Below is the flow: 1. `file-removed` event is fired from component with the deleted file 2. Initiate the delete request to your backend API and set the status of the relevant files as `LOADING` in `uploadResponse` property -3. Once the API request in completed, delete the file object from `uploadResponse` property +3. Once the API request is completed, delete the file object from `uploadResponse` property ```js preview-story export const uploadWithoutFormSubmit = () => { @@ -324,8 +323,8 @@ export const uploadWithoutFormSubmit = () => { diff --git a/packages/ui/components/input-file/src/LionInputFile.js b/packages/ui/components/input-file/src/LionInputFile.js index db53e5dbf..60b9bd8bc 100644 --- a/packages/ui/components/input-file/src/LionInputFile.js +++ b/packages/ui/components/input-file/src/LionInputFile.js @@ -13,7 +13,7 @@ import { DuplicateFileNames, IsAcceptedFile } from './validators.js'; * @typedef {import('lit').RenderOptions} RenderOptions * @typedef {import('../types/input-file.js').InputFile} InputFile * @typedef {import('../types/input-file.js').SystemFile} SystemFile - * @typedef {import('../types/input-file.js').FileSelectResponse} FileSelectResponse + * @typedef {import('../types/input-file.js').UploadResponse} UploadResponse */ /** @@ -47,7 +47,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) maxFileSize: { type: Number, attribute: 'max-file-size' }, enableDropZone: { type: Boolean, attribute: 'enable-drop-zone' }, uploadOnSelect: { type: Boolean, attribute: 'upload-on-select' }, - _fileSelectResponse: { type: Array, state: false }, + uploadResponse: { type: Array, state: false }, _selectedFilesMetaData: { type: Array, state: true }, }; } @@ -148,14 +148,14 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) */ this._selectedFilesMetaData = []; /** - * @type {FileSelectResponse[]} + * @type {UploadResponse[]} */ // TODO: make readonly? - this._fileSelectResponse = []; + this.uploadResponse = []; /** * @private */ - this.__initialFileSelectResponse = this._fileSelectResponse; + this.__initialUploadResponse = this.uploadResponse; // TODO: public default booleans are always false this.uploadOnSelect = false; this.multiple = false; @@ -178,7 +178,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) connectedCallback() { super.connectedCallback(); - this.__initialFileSelectResponse = this._fileSelectResponse; + this.__initialUploadResponse = this.uploadResponse; this._inputNode.addEventListener('change', this._onChange); this._inputNode.addEventListener('click', this._onClick); @@ -237,7 +237,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) reset() { super.reset(); this._selectedFilesMetaData = []; - this._fileSelectResponse = this.__initialFileSelectResponse; + this.uploadResponse = this.__initialUploadResponse; this.modelValue = []; // TODO: find out why it stays dirty this.dirty = false; @@ -250,7 +250,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) */ clear() { this._selectedFilesMetaData = []; - this._fileSelectResponse = []; + this.uploadResponse = []; this.modelValue = []; } @@ -383,9 +383,9 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) * 1. It is invoked from the file-removed event handler. * 2. There is a mismatch between the selected files and files on UI. */ - if (changedProperties.has('_fileSelectResponse')) { + if (changedProperties.has('uploadResponse')) { if (this._selectedFilesMetaData.length === 0) { - this._fileSelectResponse.forEach(preResponse => { + this.uploadResponse.forEach(preResponse => { const file = { systemFile: { name: preResponse.name, @@ -404,12 +404,12 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) } this._selectedFilesMetaData.forEach(file => { if ( - !this._fileSelectResponse.some(response => response.name === file.systemFile.name) && + !this.uploadResponse.some(response => response.name === file.systemFile.name) && this.uploadOnSelect ) { this.__removeFileFromList(file); } else { - this._fileSelectResponse.forEach(response => { + this.uploadResponse.forEach(response => { if (response.name === file.systemFile.name) { // eslint-disable-next-line no-param-reassign file.response = response; @@ -567,7 +567,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) const newFiles = this.__computeNewAddedFiles(Array.from(selectedFiles)); if (!this.multiple && newFiles.length > 0) { this._selectedFilesMetaData = []; - this._fileSelectResponse = []; + this.uploadResponse = []; } /** @@ -579,8 +579,8 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) fileObj = new FileHandle(selectedFile, this._acceptCriteria); if (fileObj.failedProp?.length) { this._handleErroredFiles(fileObj); - this._fileSelectResponse = [ - ...this._fileSelectResponse, + this.uploadResponse = [ + ...this.uploadResponse, { name: fileObj.systemFile.name, status: 'FAIL', @@ -589,8 +589,8 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) }, ]; } else { - this._fileSelectResponse = [ - ...this._fileSelectResponse, + this.uploadResponse = [ + ...this.uploadResponse, { name: fileObj.systemFile.name, status: 'SUCCESS', @@ -773,7 +773,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) detail: { removedFile, status: removedFile.status, - _fileSelectResponse: removedFile.response, + uploadResponse: removedFile.response, }, }), ); diff --git a/packages/ui/components/input-file/src/LionSelectedFileList.js b/packages/ui/components/input-file/src/LionSelectedFileList.js index 030455dfa..9b2929d7f 100644 --- a/packages/ui/components/input-file/src/LionSelectedFileList.js +++ b/packages/ui/components/input-file/src/LionSelectedFileList.js @@ -81,7 +81,7 @@ export class LionSelectedFileList extends LocalizeMixin(ScopedElementsMixin(LitE detail: { removedFile, status: removedFile.status, - _fileSelectResponse: removedFile.response, + uploadResponse: removedFile.response, }, }), ); 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 5d2e99f2c..6f3f38f60 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 @@ -14,7 +14,7 @@ const fixture = /** @type {(arg: TemplateResult|string) => Promise { // eslint-disable-next-line no-param-reassign - el._fileSelectResponse = [...ev.detail.newFiles]; + el.uploadResponse = [...ev.detail.newFiles]; }; function mimicSelectFile( @@ -74,7 +74,7 @@ describe('lion-input-file', () => { const fileListChangedEvent = await fileListChangedEventPromise; // @ts-expect-error [allow-protected-in-tests] expect(el._selectedFilesMetaData.length).to.equal(1); - expect(el._fileSelectResponse.length).to.equal(1); + expect(el.uploadResponse.length).to.equal(1); expect(fileListChangedEvent).to.exist; expect(fileListChangedEvent.detail.newFiles.length).to.equal(1); @@ -100,7 +100,7 @@ describe('lion-input-file', () => { await el.updateComplete; // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData.length).to.equal(1); - expect(el._fileSelectResponse.length).to.equal(1); + expect(el.uploadResponse.length).to.equal(1); // when cancel is clicked, native input value is blank which means modelValue is blank el.modelValue = []; @@ -174,7 +174,7 @@ describe('lion-input-file', () => { expect(el._selectedFilesMetaData[0].validationFeedback).to.exist; // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData[0].status).to.equal('FAIL'); - expect(el._fileSelectResponse[0].status).to.equal('FAIL'); + expect(el.uploadResponse[0].status).to.equal('FAIL'); }); it('error message should use main type when "/*" is used', async () => { @@ -299,7 +299,7 @@ describe('lion-input-file', () => { expect(el._selectedFilesMetaData[0].validationFeedback).to.exist; // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData[0].status).to.equal('FAIL'); - expect(el._fileSelectResponse[0].status).to.equal('FAIL'); + expect(el.uploadResponse[0].status).to.equal('FAIL'); }); it('error message should add the file extension to the validator message', async () => { @@ -377,7 +377,7 @@ describe('lion-input-file', () => { expect(el._selectedFilesMetaData[0].validationFeedback).to.exist; // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData[0].status).to.equal('FAIL'); - expect(el._fileSelectResponse[0].status).to.equal('FAIL'); + expect(el.uploadResponse[0].status).to.equal('FAIL'); }); it('error message should show only the max file size if no type/extension restrictions are defined', async () => { @@ -429,7 +429,7 @@ describe('lion-input-file', () => { expect(el._selectedFilesMetaData.length).to.equal(2); // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData[0].status).to.equal('FAIL'); - expect(el._fileSelectResponse[0].status).to.equal('FAIL'); + expect(el.uploadResponse[0].status).to.equal('FAIL'); expect(fileListChangedEvent.detail.newFiles.length).to.equal(1); }); @@ -543,9 +543,9 @@ describe('lion-input-file', () => { // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData[1].systemFile.name).to.equal('bar.txt'); - expect(el._fileSelectResponse.length).to.equal(2); - expect(el._fileSelectResponse[0].name).to.equal('foo.txt'); - expect(el._fileSelectResponse[1].name).to.equal('bar.txt'); + expect(el.uploadResponse.length).to.equal(2); + expect(el.uploadResponse[0].name).to.equal('foo.txt'); + expect(el.uploadResponse[1].name).to.equal('bar.txt'); }); it('should add new files and retain previous files', async () => { @@ -664,10 +664,10 @@ describe('lion-input-file', () => { el = await fixture(html``); }); - it('should set _fileSelectResponse data to _selectedFilesMetaData for rendering error and status', async () => { + it('should set uploadResponse data to _selectedFilesMetaData for rendering error and status', async () => { mimicSelectFile(el, [file]); - el._fileSelectResponse = [{ name: 'foo.txt', status: 'LOADING', errorMessage: '500' }]; + el.uploadResponse = [{ name: 'foo.txt', status: 'LOADING', errorMessage: '500' }]; await el.updateComplete; @@ -735,8 +735,8 @@ describe('lion-input-file', () => { expect(el.hasFeedbackFor.includes('error')).to.be.true; }); - it('reset method should remove File from modelValue but keep _fileSelectResponse', async () => { - const _fileSelectResponse = [ + it('reset method should remove File from modelValue but keep uploadResponse', async () => { + const uploadResponse = [ { name: 'file1.txt', status: 'SUCCESS', @@ -745,14 +745,11 @@ describe('lion-input-file', () => { }, ]; const el = await fixture(html` - + `); // @ts-expect-error [allow-protected-in-test] - el._fileSelectResponse = _fileSelectResponse; + el.uploadResponse = uploadResponse; await el.updateComplete; mimicSelectFile(el, [file]); @@ -762,11 +759,11 @@ describe('lion-input-file', () => { expect(el.modelValue).to.deep.equal([]); // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData).to.deep.equal([]); - expect(el._fileSelectResponse).to.deep.equal(_fileSelectResponse); + expect(el.uploadResponse).to.deep.equal(uploadResponse); }); - it('clear method should remove File from modelValue and _fileSelectResponse', async () => { - const _fileSelectResponse = [ + it('clear method should remove File from modelValue and uploadResponse', async () => { + const uploadResponse = [ { name: 'file1.txt', status: 'SUCCESS', @@ -777,7 +774,7 @@ describe('lion-input-file', () => { const el = await fixture(html` `); // @ts-expect-error [allow-protected-in-test] - el._fileSelectResponse = _fileSelectResponse; + el.uploadResponse = uploadResponse; setTimeout(() => { mimicSelectFile(el, [file]); @@ -785,7 +782,7 @@ describe('lion-input-file', () => { await oneEvent(el, 'file-list-changed'); await el.clear(); expect(el.modelValue).to.deep.equal([]); - expect(el._fileSelectResponse).to.deep.equal([]); + expect(el.uploadResponse).to.deep.equal([]); // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData).to.deep.equal([]); }); @@ -796,7 +793,7 @@ describe('lion-input-file', () => { * @type {LionInputFile} */ let el; - const _fileSelectResponse = [ + const uploadResponse = [ { name: 'file1.txt', status: 'SUCCESS', @@ -816,12 +813,12 @@ describe('lion-input-file', () => { `); // @ts-expect-error [allow-protected-in-test] - el._fileSelectResponse = _fileSelectResponse; + el.uploadResponse = uploadResponse; await el.updateComplete; }); - it('should update the _selectedFilesMetaData according to _fileSelectResponse', () => { + it('should update the _selectedFilesMetaData according to uploadResponse', () => { // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData.length).to.equal(2); // @ts-expect-error [allow-protected-in-test] @@ -862,7 +859,7 @@ describe('lion-input-file', () => { detail: { removedFile, status: removedFile.status, - _fileSelectResponse: removedFile.response, + uploadResponse: removedFile.response, }, }), ); @@ -874,7 +871,7 @@ describe('lion-input-file', () => { removeFileSpy.restore(); }); - it('should fire file-removed event with _fileSelectResponse in the details', async () => { + it('should fire file-removed event with uploadResponse in the details', async () => { /** * @type {Partial} */ @@ -908,7 +905,7 @@ describe('lion-input-file', () => { }, }); expect(removeFileEvent.detail.status).to.deep.equal('FAIL'); - expect(removeFileEvent.detail._fileSelectResponse).to.deep.equal({ + expect(removeFileEvent.detail.uploadResponse).to.deep.equal({ name: 'file2.txt', status: 'FAIL', }); @@ -1026,14 +1023,14 @@ describe('lion-input-file', () => { }, }); expect(removeFileEvent.detail.status).to.deep.equal('SUCCESS'); - expect(removeFileEvent.detail._fileSelectResponse).to.deep.equal({ + expect(removeFileEvent.detail.uploadResponse).to.deep.equal({ name: 'foo.txt', status: 'SUCCESS', }); }); - it('should remove file from displayed list if not available in _fileSelectResponse array', async () => { - const _fileSelectResponse = [ + it('should remove file from displayed list if not available in uploadResponse array', async () => { + const uploadResponse = [ { name: 'file1.txt', status: 'SUCCESS', @@ -1052,13 +1049,13 @@ describe('lion-input-file', () => { `); // @ts-expect-error [allow-protected-in-test] - el._fileSelectResponse = _fileSelectResponse; + el.uploadResponse = uploadResponse; await el.updateComplete; // assertion for displayed file list to be same // @ts-expect-error [allow-protected-in-test] expect(el._selectedFilesMetaData.length).to.equal(2); - el._fileSelectResponse = [ + el.uploadResponse = [ { name: 'file1.txt', status: 'SUCCESS', @@ -1084,7 +1081,7 @@ describe('lion-input-file', () => { }); it('is accessible when a file is selected', async () => { - const _fileSelectResponse = [ + const uploadResponse = [ { name: 'file1.txt', status: 'SUCCESS', @@ -1095,13 +1092,13 @@ describe('lion-input-file', () => { const el = await fixture(html` `); // @ts-expect-error [allow-protected-in-test] - el._fileSelectResponse = _fileSelectResponse; + el.uploadResponse = uploadResponse; await expect(el).to.be.accessible(); }); it('is accessible with an selected file and disabled', async () => { - const _fileSelectResponse = [ + const uploadResponse = [ { name: 'file1.txt', status: 'SUCCESS', @@ -1113,7 +1110,7 @@ describe('lion-input-file', () => { const el = await fixture(html` `); // @ts-expect-error [allow-protected-in-test] - el._fileSelectResponse = _fileSelectResponse; + el.uploadResponse = uploadResponse; await expect(el).to.be.accessible(); }); @@ -1134,7 +1131,7 @@ describe('lion-input-file', () => { }); it('select-button has aria-describedby set to the help-text, selected list and the feedback message', async () => { - const _fileSelectResponse = [ + const uploadResponse = [ { name: 'file1.txt', status: 'SUCCESS', @@ -1147,7 +1144,7 @@ describe('lion-input-file', () => { `); // @ts-expect-error [allow-protected-in-test] - el._fileSelectResponse = _fileSelectResponse; + el.uploadResponse = uploadResponse; // @ts-expect-error [allow-protected-in-test] expect(el._buttonNode?.getAttribute('aria-describedby')).to.contain( diff --git a/packages/ui/components/input-file/test/lion-selected-file-list.test.js b/packages/ui/components/input-file/test/lion-selected-file-list.test.js index 4a9bb2ecb..e5e0e779a 100644 --- a/packages/ui/components/input-file/test/lion-selected-file-list.test.js +++ b/packages/ui/components/input-file/test/lion-selected-file-list.test.js @@ -167,7 +167,7 @@ describe('lion-selected-file-list', () => { }, }); expect(removeFileEvent.detail.status).to.deep.equal('SUCCESS'); - expect(removeFileEvent.detail._fileSelectResponse).to.deep.equal({ + expect(removeFileEvent.detail.uploadResponse).to.deep.equal({ name: 'foo.txt', status: 'SUCCESS', }); diff --git a/packages/ui/components/input-file/types/input-file.d.ts b/packages/ui/components/input-file/types/input-file.d.ts index 8e5e18383..048554e0e 100644 --- a/packages/ui/components/input-file/types/input-file.d.ts +++ b/packages/ui/components/input-file/types/input-file.d.ts @@ -12,7 +12,7 @@ export type InputFile = { downloadUrl?: string; errorMessage?: FeedbackMessage.message; failedProp?: Array; - response?: FileSelectResponse; + response?: UploadResponse; systemFile: Partial; validationFeedback?: Array; } & FileBasics & Partial; @@ -21,11 +21,11 @@ export type SystemFile = { downloadUrl?: string; errorMessage?: FeedbackMessage.message; failedProp?: Array; - response?: FileSelectResponse; + response?: UploadResponse; } & FileBasics & Partial; -export type FileSelectResponse = { +export type UploadResponse = { downloadUrl?: string; errorMessage?: FeedbackMessage.message; id?: string;