Skip to content

Commit

Permalink
bugfix + address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
gossi committed Jan 4, 2022
1 parent 88f651b commit 8e8ef49
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 44 deletions.
74 changes: 33 additions & 41 deletions addon/queue.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { action } from '@ember/object';
import { next } from '@ember/runloop';
import { modifier, ModifierArgs } from 'ember-modifier';
import { tracked, TrackedArray } from 'tracked-built-ins';
import { action } from '@ember/object';

import UploadFile, { FileState, FileSource } from './file';
import { TrackedArray, TrackedSet } from 'tracked-built-ins';
import UploadFile, { FileSource, FileState } from './file';
import FileQueueService from './services/file-queue';

export interface SelectFileModifierArgs extends ModifierArgs {
Expand Down Expand Up @@ -55,33 +54,28 @@ export default class Queue {
/** The FileQueue service. */
fileQueue: FileQueueService;

// @tracked private distinctFiles: Set<File> = new Set();
// Must uses `TrackedArray()` until this one is solved:
// https://github.com/tracked-tools/tracked-built-ins/issues/225
files: UploadFile[] = new TrackedArray([]);

// /**
// * The list of files in the queue. This automatically gets
// * flushed when all the files in the queue have settled.
// *
// * @remarks
// * Note that files that have failed need to be manually
// * removed from the queue. This is so they can be retried
// * without resetting the state of the queue, orphaning the
// * file from its queue. Upload failures can happen due to a
// * timeout or a server response. If you choose to use the
// * `abort` method, the file will fail to upload, but will
// * be removed from the requeuing proccess, and will be
// * considered to be in a settled state.
// */
// get files(): File[] {
// console.log('queue.files', this.distinctFiles);

// return [...this.distinctFiles.values()];
// }
#distinctFiles: Set<UploadFile> = new TrackedSet();

/**
* The list of files in the queue. This automatically gets
* flushed when all the files in the queue have settled.
*
* @remarks
* Note that files that have failed need to be manually
* removed from the queue. This is so they can be retried
* without resetting the state of the queue, orphaning the
* file from its queue. Upload failures can happen due to a
* timeout or a server response. If you choose to use the
* `abort` method, the file will fail to upload, but will
* be removed from the requeuing proccess, and will be
* considered to be in a settled state.
*/
get files(): UploadFile[] {
return [...this.#distinctFiles.values()];
}

// @TODO: Is this needed? I think, this is what each dropzone needs to manage
_dropzones = tracked([]);
_dropzones = new TrackedArray([]);

/**
* The total size of all files currently being uploaded in bytes.
Expand Down Expand Up @@ -127,12 +121,6 @@ export default class Queue {
this.fileQueue = fileQueue;
}

// @TODO this is called from nowhere ?!?
// destroy() {
// this.distinctFiles.forEach((file) => (file.queue = undefined));
// this.distinctFiles.clear();
// }

addListener(listener: QueueListener) {
this.#listeners.add(listener);
}
Expand All @@ -153,13 +141,12 @@ export default class Queue {
*/
@action
add(file: UploadFile) {
if (this.files.includes(file)) {
if (this.#distinctFiles.has(file)) {
return;
}

file.queue = this;
// this.distinctFiles.add(file);
this.files.push(file);
this.#distinctFiles.add(file);

for (const listener of this.#listeners) {
listener.fileAdded?.(file);
Expand All @@ -172,12 +159,12 @@ export default class Queue {
*/
@action
remove(file: UploadFile) {
if (!this.files.includes(file)) {
if (!this.#distinctFiles.has(file)) {
return;
}

file.queue = undefined;
this.files.splice(this.files.indexOf(file), 1);
this.#distinctFiles.delete(file);

for (const listener of this.#listeners) {
listener.fileRemoved?.(file);
Expand Down Expand Up @@ -253,7 +240,7 @@ export default class Queue {

if (allFilesHaveSettled) {
this.files.forEach((file) => (file.queue = undefined));
this.files = [];
this.#distinctFiles.clear();
}
}

Expand Down Expand Up @@ -284,6 +271,11 @@ export default class Queue {
}

named.filesSelected?.(selectedFiles);

// this will reset the input, so the _same_ file can be picked again
// Without, the `change` event wouldn't be fired, as it is still the same
// value
element.value = '';
};
element.addEventListener('change', changeHandler);

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"postpack": "ember ts:clean"
},
"dependencies": {
"@ember/render-modifiers": "^2.0.0",
"@ember/test-waiters": "^3.0.0",
"@glimmer/component": "^1.0.4",
"@glimmer/tracking": "^1.0.4",
Expand All @@ -61,7 +62,6 @@
},
"devDependencies": {
"@ember/optional-features": "^2.0.0",
"@ember/render-modifiers": "^2.0.0",
"@ember/test-helpers": "^2.4.2",
"@embroider/test-setup": "^0.47.0",
"@types/ember-qunit": "^3.4.15",
Expand Down
61 changes: 59 additions & 2 deletions tests/helpers/file-queue-helper-test.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render, click } from '@ember/test-helpers';
import { render, click, settled, waitFor } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';

import { selectFiles } from 'ember-file-upload/test-support';
import { DEFAULT_QUEUE } from 'ember-file-upload/services/file-queue';

import { upload as uploadHandler } from 'ember-file-upload/mirage';
import setupMirage from 'ember-cli-mirage/test-support/setup-mirage';

module('Integration | Helper | file-queue', function (hooks) {
setupRenderingTest(hooks);
setupMirage(hooks);

test('filter is triggered when selecting files', async function (assert) {
this.filter = (file) => assert.step(`filter: ${file.name}`);
Expand Down Expand Up @@ -42,8 +46,14 @@ module('Integration | Helper | file-queue', function (hooks) {
`);

await selectFiles('input[type=file]', new File([], 'dingus.txt'));
await selectFiles('input[type=file]', new File([], 'dingus.txt'));
await selectFiles('input[type=file]', new File([], 'dingus.png'));

assert.verifySteps(['files selected: dingus.txt']);
assert.verifySteps([
'files selected: dingus.txt',
'files selected: dingus.txt',
'files selected: dingus.png',
]);
});

test('falls back to default name', async function (assert) {
Expand Down Expand Up @@ -102,4 +112,51 @@ module('Integration | Helper | file-queue', function (hooks) {

assert.dom('[data-test-file]').doesNotExist();
});

test('uploading twice will manage files in queue', async function (assert) {
this.server.post(
'/folder/:id/file',
uploadHandler(function (schema, request) {
// do sth
})
);

this.uploadImage = (file) => {
return file.upload('/folder/1/file');
};

await render(hbs`
{{#let (file-queue fileAdded=this.uploadImage) as |queue|}}
{{#each queue.files as |file|}}
<span data-test-file>
{{file.name}}
</span>
{{/each}}
<label>
<input type="file" {{queue.selectFile}} hidden>
Select File
</label>
{{/let}}
`);

assert.dom('[data-test-file]').doesNotExist();

// first upload
selectFiles('input[type=file]', new File([], 'dingus.txt'));
await waitFor('[data-test-file]');
assert.dom('[data-test-file]').hasText('dingus.txt');

await settled();
assert.dom('[data-test-file]').doesNotExist();

// second upload (retention teset)
// to check `Queue.flush()` will keep auto-tracking intact
selectFiles('input[type=file]', new File([], 'dingus.txt'));
await waitFor('[data-test-file]');
assert.dom('[data-test-file]').hasText('dingus.txt');

await settled();
assert.dom('[data-test-file]').doesNotExist();
});
});

0 comments on commit 8e8ef49

Please sign in to comment.