Opened 5 years ago

Last modified 3 years ago

#12810 closed Bug

Create a type for easy progress notifiaction handling. — at Version 9

Reported by: Marek Lewandowski Owned by:
Priority: Normal Milestone: CKEditor 4.5.0 Beta
Component: General Version:
Keywords: Cc:

Description (last modified by Marek Lewandowski)

Overview

We need to create a convenient way to handle notification will display a progress for related items.

Eg. editor is waiting for 5 JSON responses. It would be nice if we'll display a progress notification like "Waiting for response 0/5".

Then when the first is done, it will be changed to "Waiting for response 1/5", updating the progress bar to 20% and so on.

http://i.imgur.com/14RBHju.png

At the end we ither show a success information, or just hide the notification.

Proposed API (simple case)

API should be very simple.

Aggregator instance would be created as follows (note CKEDITOR.plugins.notificationaggregator is a temp name):

var notification = new CKEDITOR.plugins.notificationaggregator( editor, 'Loading process {current} / {max}' );

var fooRequestDone = notification.createThread();

framework.getJson( '//foobar.dev/baz.json' )
	.done( function() {
		// Success!
	} )
	.always( fooRequestDone );

// (...)

So as we can see createThread() returns a callback that should be called in order to mark a thread as completed.

More complex case

Upload plugin would need more complex solution, as it is preferred to give end user constant feedback of the process progression.

Consider uploading three 200 Mb files.

But problem with files is that they might have different size, and our progress needs to be reliable. We could workaround it by giving bytes count to the API.

var notification = new ComplexAggregator( editor, 'Uploading file {current} / {max}' ),
	thread;

xhr.upload.addEventListener( 'loadstart', function() {
	// Create a new thread once upload was started.
	thread = notification.createThread( e.total );
} );

xhr.upload.addEventListener( 'progress', function() {
	// Update the progress with every progress tick.
	thread.update( e.loaded );
} );

xhr.upload.addEventListener( 'loadend', function() {
	// Make sure that thread was marked as done. The same would do thread.update( e.total );
	thread.done();
} );

So the object would expose 2 properties - update and done.

As a side note:

This case shows that internal aggregator update method should be throttled, becuase considering eg. 10 files upload, there would be a ridiculus ammount of calls to the ui update funciton.

Different title if only one item remaining

Initialy I thought about possibility to displayh a different title if only one element remains.

Eg. "Uploading files... 1/5" 3 files later would print "Uploading 'myCustomImage.png'... 43%" rather than "Uploading files... 4/5".

This functionallity would be totally optional, so if one want to use it he needs only to specify extra parameter to the constructor, like so:

var notification = new ComplexAggregator( editor, 'Uploading file {current} / {max}', 'Uploading \'{name}\'... {percentage}%' );

But at the end I was wondering if the effor is really worth it? Because it will also require us to modify createThread() method (because we'll need to keep track of the name).

Related tickes:

Change History (9)

comment:1 Changed 5 years ago by Marek Lewandowski

Description: modified (diff)

comment:2 Changed 5 years ago by Marek Lewandowski

Description: modified (diff)

comment:3 Changed 5 years ago by Frederico Caldeira Knabben

Instead of "thread", what about "task"?

As for different message if only one "task", I don't think it is necessary. It just makes the API harder to use right.

I would focus on having generic messages that can fit on one or many tasks, added by a separate counter (included by the notification plugin itself). For example, the developer passes "Uploading file" to the API. If it is one file only, you see it as is. If it is more than one, you see "Uploading file (1 of 10)".

The " (1 of 10)" should be localizable, with a localization string that looks like this: '%m (%c of %t)'.

Btw, it is taken in consideration the possibility of adding "tasks" during the progress, right? I mean, if we have "3 of 5" we can keep adding new tasks, so it'll change to "3 of 6" and so on, right?

comment:4 Changed 5 years ago by Piotr Jasiun

Error handling

Notification aggregator should handle errors: if 4 files are downloading and there is a problem with 1 of them, then the final message should be "Downloaded files 3/3" (not 4/4). Even if there is an warning message below the main message that all files have been downloaded is misleading. So the thread API should have 'cancel' method:

aggregator = new CKEDITOR.plugins.notification.aggregator( editor );
t1 = aggregator.createThread();
t2 = aggregator.createThread();
t3 = aggregator.createThread();

t1.done();   // message: 1/3 done
t2.cancel(); // message: 1/2 done
t3.done();   // message: 2/2 done

Progress and weights

Definitely it should be possible to show partial progress. Also there may be smaller and bigger files uploaded. So the idea is, that createThread take a total parameter (1 by default) and thread has update method so API usage will looks like this:

aggregator = new CKEDITOR.plugins.notification.aggregator( editor );
t1 = aggregator.createThread( { total: 20 } ); // the 'size' of the first thread is 20
t2 = aggregator.createThread( { total: 80 } ); // the 'size' of the first thread is 80

t1.update( 5 );  //  25% of the first  file is done,  5% of general progress
t1.done();       // 100% of the first  file is done, 20% of general progress
t2.update( 30 ); //  30% of the second file is done, 80% of general progress

If user do not want to care about file sizes he get skip total parameter and every done will upgrade total by 1.

Messages

In the most common case (and most often) for the upload, the one file will be uploaded and then message for that file should be:

Uploading 'foo.pg' 84%...

instead of:

Uploading 0/1...

But if user drop 2 files aggregator should be used. Also it may happens that the second upload will start before the first will be done, in such case single message should be replaced with agreggated.

The idea is to use always aggregator, but if it contains only one thread the message of that thread will be displayed:

var aggregator = new CKEDITOR.plugins.notification.aggregator( editor, 'General msg %' );
var t1 = aggregator.createThread( { msg: 't1 specific msg' } ); // message: t1 specific msg
var t2 = aggregator.createThread( { msg: 't2 specific msg' } ); // message: General msg 0/2

t2.done(); // message: General msg 1/2
t1.done(); // message: General msg 2/2

So if aggregator contains more than one thread or there is no specific message then general message is used.

Message regexp

Some extra parameters should be passed to the message like:

  • done percentage ('Downloaded 84%...').
  • total number of threads ('Downloading 3 files...').
  • number of done threads ('Downloaded 2 of 3 files...').

It should be also possible to show different message when everything is done: type of the notification should be 'success' the message should change:

var aggregator = new CKEDITOR.plugins.notification.aggregator( editor, { message: 'Downloading %...', doneMessage: 'Dowloaded %!' } );
var t1 = aggregator.createThread(); // message: Downloading 0/1...
var t2 = aggregator.createThread(); // message: Downloading 0/2...

t1.done(); // message: Downloading 1/2...
t2.done(); // message: Downloaded 2/2!

Get aggregator progress

It should be also possible to get the progress of the aggregator, because if I want to start a new thread I can add it to the current aggregator or create a new one, if the current is done. It makes no sense to show: Dowloading 5/6... if the previous 5 download have been done long ago:

if( aggregator.isDone() ) {
    aggregator = new CKEDITOR.plugins.notification.aggregator( editor );
}
t1 = aggregator.createThread();

comment:5 in reply to:  description ; Changed 5 years ago by Piotr Jasiun

Replying to m.lewandowski:

As a side note:

This case shows that internal aggregator update method should be throttled, becuase considering eg. 10 files upload, there would be a ridiculus ammount of calls to the ui update funciton.

Not that ridiculous. As I tested it the progress event is fired rarely. Only once during testing upload I managed to get 2 progress events during one upload.

comment:6 in reply to:  3 Changed 5 years ago by Piotr Jasiun

Replying to fredck:

Instead of "thread", what about "task"?

+1

comment:7 Changed 5 years ago by Piotr Jasiun

And one more thing: part of the features may be done as a part of #12724. Now the most important is to create a API in the scalable way. E.g. if aggregator.createTask(); return function instead of object we would not be able to add more functions to it in the future.

comment:8 in reply to:  5 Changed 5 years ago by Marek Lewandowski

Replying to fredck:

Instead of "thread", what about "task"?

Task sounds good for me.

I would focus on having generic messages that can fit on one or many tasks (...)

This simplification might be actually good for the time being.

Btw, it is taken in consideration the possibility of adding "tasks" during the progress, right? I mean, if we have "3 of 5" we can keep adding new tasks, so it'll change to "3 of 6" and so on, right?

Yes it's already implemented that way.

Replying to pjasiun:

And one more thing: part of the features may be done as a part of #12724.

I think it make sense to make both classes within a single ticket. Then the other ticket will be simply about integrating it into a upload widget.

Replying to pjasiun:

Error handling

Notification aggregator should handle (...)

Not sure if anyone will care about such a detail.

Progress and weights

Definitely it should be possible to show partial progress.

Yes I agree that this is a must have feature in case of more complex implementation.

Messages

In the most common case (and most often) for the (...)

Well it would be an ok feature to have, but it makes interface more complex.

It comes at a cost of requiring user to put an extra string to the constructor, and adding an extra string each time he registers a thread.

Then also we need to make sure that we describe that relation clearly in the docs.

Message regexp

(...)

  1. These parameters are a must have in templates, and they're already there.

1.

It should be also possible to show different message when everything is done: type of the notification should be 'success' the message should change:

This will be handled either by overriding finish method or adding an event listener.

  1. Not sure what do you mean by regexp?

Get aggregator progress

(...)

There's already a function that tells whether the process is finished or not.

Replying to pjasiun:

Not that ridiculous. As I tested it the progress event is fired rarely. Only once during testing upload I managed to get 2 progress events during one upload.

Well, are you sure about this? I did test 3 browsers today, and chrome indeed doesn't spam with progress events, consider other browsers:

  • FF - 11 events per second (but it varies, I got even 15+ on lower spec machine)
  • IE11 - 9.7 events per second

And this is only for one FormData at a time (see jsfiddle).

Last edited 5 years ago by Marek Lewandowski (previous) (diff)

comment:9 Changed 5 years ago by Marek Lewandowski

Description: modified (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy