fire an event on the FileRepository plugin when a new loader is created

[robclancy]

Currently uploading is very restricted due to a lack of events that you usually get. We have no way to do anything with an upload after it is uploaded. Like this issue. ckeditor/ckeditor5#2833

I wanted to add a bunch of events and also tried delegating some from the loader. I was looking at events used in ckeditor 4 and there are a lot there I would like, however one that basically means we have the power to do what we need, https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_fileTools_uploadRepository.html#event-instanceCreated.

So this PR is basically to add that same event from ckeditor4 which means you can get access to the repsonse.

This is how I am using it.

  listenToUpload(editor) {
    let fileRepository = editor.plugins.get('FileRepository');

    if (fileRepository) {
      fileRepository.on('loaderCreated', (event, loader) => {
        loader.on('change:uploadResponse', () => {
          if (loader.uploadResponse) {
            this.editorUpload(loader.uploadResponse);
          }
        });
      });
    }
  }

[robclancy]

I've just realized because of how SimpleUploadAdapter resolves the response you still don't get the full response. So for the linked issue you still need to extend that adapter to handle the response correctly. But at least you can do that now without hacking in events into the adapter which shouldn't be worried about the "outside world".

[robclancy]

Here is where we had to extend simple adapter so we can get our real response.
https://github.com/postedin/ember-ckeditor/blob/master/addon/simple-upload-adapter.js#L13

And here is the listener being used.
https://github.com/postedin/ember-ckeditor/blob/master/addon/components/ckeditor/component.js#L113

[robclancy]

28 days. A single line change. No response. I want to PR other things but they will have a lot more changed... what is the point if I can't even get this looked at?

[oleq]

Hi, @robclancy!

Sorry you had to wait for so long but the end of the year is a hot period for our company and we had different priorities.

PR

As for the PR, it totally makes sense to me. I checked it using the following code

editor.plugins.get( 'FileRepository' ).on( 'loaderCreated', ( evt, loader ) => {
	loader.on( 'change:uploadResponse', ( evt, name, value ) => {
		console.log( 'response', loader, value );
	} );

	loader.on( 'change:status', ( evt, name, value ) => {
		console.log( 'status', loader, value );
	} );
} );

and the communication with the SimpleUploadAdapter enabled looks good

so it would be great if you could tell us more about the problems you mentioned

I've just realized because of how SimpleUploadAdapter resolves the response you still don't get the full response.

TODOs

There are still a few things necessary before this PR can be merged:

• Docs – the event needs public documentation similar to others.
• Tests – we require 100% test coverage for any piece of code in the editor, especially a public event.
• CLA – I noticed it still awaits a signature.

For more information, check out our "Contributing" guide.

Let me know if you are going to work on this PR any further. If not, we'll try to take it over and proceed on our own.

Thanks!

---

@Reinmar: While this PR makes sense on the upload logic level it is still not the best UX because you don't get any access to the results in the model. Most of the people are going to use ImageUpload (which uses ImageUploadEditing) and I think the editing plugin should fire some events when the image first appears in the content (preview) and when it is updated with the upload URL from the server. Naturally, both should pass a reference to the image in the model. WDYT?

[robclancy]

https://github.com/postedin/ember-ckeditor/blob/master/addon/simple-upload-adapter.js#L36
vs
https://github.com/ckeditor/ckeditor5-upload/blob/master/src/adapters/simpleuploadadapter.js#L179

The current implementation will only have a url or urls extracted from the response.

I will get back to this properly later.