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.<type> to Array<type>
       - 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 <gerjan.van.geest@ing.com>
Co-authored-by: Thijs Louisse <Thijs.Louisse@ing.com>
This commit is contained in:
Danny Moerkerke 2023-06-07 12:57:34 +02:00 committed by GitHub
parent 6893421779
commit 8b9c8050e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 66 additions and 70 deletions

View file

@ -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
>
</lion-input-file>
```
@ -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 = () => {
<lion-input-file
label="Upload"
name="upload"
.multiple="${true}"
.uploadOnSelect="${true}"
multiple
upload-on-select
@file-removed="${ev => {
ev.target.uploadResponse[
ev.target.uploadResponse.findIndex(file => file.name === ev.detail.uploadResponse.name)
@ -399,7 +398,7 @@ export const dragAndDrop = () => {
accept=".png"
max-file-size="1024000"
enable-drop-zone
.multiple="${true}"
multiple
.validators="${[new Required()]}"
>
</lion-input-file>

View file

@ -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,
},
}),
);

View file

@ -81,7 +81,7 @@ export class LionSelectedFileList extends LocalizeMixin(ScopedElementsMixin(LitE
detail: {
removedFile,
status: removedFile.status,
_fileSelectResponse: removedFile.response,
uploadResponse: removedFile.response,
},
}),
);

View file

@ -14,7 +14,7 @@ const fixture = /** @type {(arg: TemplateResult|string) => Promise<LionInputFile
const filesListChanged = (/** @type {LionInputFile} */ el, /** @type { CustomEvent } */ ev) => {
// 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`<lion-input-file accept="text/plain"></lion-input-file>`);
});
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`
<lion-input-file
label="Select"
._fileSelectResponse="${_fileSelectResponse}"
></lion-input-file>
<lion-input-file label="Select" .uploadResponse="${uploadResponse}"></lion-input-file>
`);
// @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` <lion-input-file label="Select"></lion-input-file> `);
// @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<InputFile>}
*/
@ -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` <lion-input-file label="Select"></lion-input-file>`);
// @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` <lion-input-file label="Select" disabled></lion-input-file> `);
// @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(

View file

@ -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',
});

View file

@ -12,7 +12,7 @@ export type InputFile = {
downloadUrl?: string;
errorMessage?: FeedbackMessage.message;
failedProp?: Array<string|number>;
response?: FileSelectResponse;
response?: UploadResponse;
systemFile: Partial<SystemFile>;
validationFeedback?: Array<FeedbackMessage>;
} & FileBasics & Partial<File>;
@ -21,11 +21,11 @@ export type SystemFile = {
downloadUrl?: string;
errorMessage?: FeedbackMessage.message;
failedProp?: Array<string|number>;
response?: FileSelectResponse;
response?: UploadResponse;
} & FileBasics & Partial<File>;
export type FileSelectResponse = {
export type UploadResponse = {
downloadUrl?: string;
errorMessage?: FeedbackMessage.message;
id?: string;