Opened 10 years ago
Last modified 7 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 )
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.
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 10 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:3 follow-up: 6 Changed 10 years ago by
comment:4 Changed 10 years ago by
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 follow-up: 8 Changed 10 years ago by
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:7 Changed 10 years ago by
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 Changed 10 years ago by
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
(...)
- 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.
- 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).
comment:9 Changed 10 years ago by
Description: | modified (diff) |
---|
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?