Introduce the "insertImage" command

[jodator]

Right now we only have "imageUpload" command that uploads & inserts an image. Other features (ie file browser features) might do partially the same - insert an image (by its URL).

We could introduce the "insertImage" command that will insert an image for given URL in document selection. It will use findOptimalPostion() algorithm.

editor.execute( 'insertImage', { src: '/some/path/to/image.jpg' } );

[jodator]

@Reinmar Just a quick question:

1. InsertImage
2. ImageInsert (we have ImageUpload)

[Reinmar]

OMG, I knew we'll end up in this place :D Flip the coin, I guess.

But to be serious – imageInsert (we're talking about a command, right?) would be incorrect – it should be imageInsertion. The "upload" works as a noun in imageUpload(Feature). But imageInsertion looks ridiculous... so imageInsert. Yeah... let's go with this one and treat it as a compound of namespace + action. Let's be consistent locally (so with imageUpload).

[scofalik]

Why not insertImage? (we could rename imageUpload to uploadImage too)

[Reinmar]

Hm... I think I recall that we anticipated this problem and we agreed that imageUpload is indeed a namespace + action. I had a feeling we were there already.

[Reinmar]

Why not insertImage?

Because we already have imageUpload. Which we have because the feature is called ImageUpload (it would be bad if feature was called differently than the button). We also have imageStyle:* and imageTextAlternative (plus, plugins called the same way).

We had a very long discussion about naming ~1 year ago and we agreed that a single naming convention doesn't fit all cases. We have features like undo/redo/bold (noun or verb) but also blockQuote or heading (noun). It's a bit of a mess. You would expect that a command name is a verb, but that caused issues, so we have to be flexible and as a rule of thumb consistent locally (within a package) rather than trying to be super consistent globally (impossible).

[jodator]

imageInsert and ImageInsertCommand to be 👯‍♂️

[jodator]

Another thing which we rather didn't do (apart from AttributeCommand which is a bit different case).

So I see that ImageUploadCommand and ImageInsertCommand will behave similarly. Mostly in the refresh() method. I would make ImageUploadCommand to extend ImageInsertCommand to not duplicate this part and rewrite the execute() part. Will this be OK?

[Reinmar]

One should not extend the other. They have different API signatures for execute(). That violates LSP. If anything, we should extract the common part and reuse it in both commands. I doubt it's worth at this stage.