Fixed image resize flashing unexpected size

[mlewand]

Suggested merge commit message (convention)

Fix: Fixed image resize behavior upon short clicking a handle without dragging. Image will no longer became full width, nor will it briefly flash an unexpected size. Closes ckeditor/ckeditor5#5189 and closes ckeditor/ckeditor5#5195.

MINOR BREAKING CHANGE: Resizer options object now also takes an editor instance.

---

Additional information

• note Since ResizerOptions now holds editor reference, it is no longer needed to keep downcastWriter so I'd also like to remove this property. I find it to be a minor breaking change, please confirm that.

Short overview of the problems:

• flickering image at the first render (#5189)
  The issue is caused by the fact that we played a little against the UI with the initial implementation.

  The resizer was applying the new width styles directly to the img DOM element, skipping view abstraction. What happened there was that imageresize plugin added a img.image_resized class to the img's view, which gives it a 100% width. Since the view knew nothing about the width attribute, it did not add it while re-rendering img. Thus was getting full width for a brief moment, until the DOM img was modified once again directly.

  Now implementation works with view, that required me to pass editor instance to the resizer so I can request for a view writer.

• committing full image width, when end user only clicks the handle (but does not move the cursor) (#5195)
  The problem is caused by the fact that commit() accesses an uninitialized state. And this occurs when user clicks the resize and releases the cursor but does not trigger mousemove event, thus the state is never initialized.

  It could be worked around in the imageresize plugin itself (just checking the state here), but the fix should be in the core and prevent commit from being issued.

Subprs:

• Fixed image resize flashing unexpected size ckeditor5-image#334

[coveralls]

Coverage remained the same at 100.0% when pulling 974c58d on i/5189 into b9cf062 on master.

[Mgsy]

LGTM.

[Reinmar]

The "view" doesn't mean that it's a view but it comes from the view so it's an adjective and should be the first word. Just like in the variable which you actually use below:

this._resizerWrapperView = viewResizerWrapper;

Next to the viewResizerWrapper you have a domResizerWrapper.

[Reinmar]

Please also fix the description of this property.

[Reinmar]

• I find it to be a minor breaking change, please confirm that.

Yup, that's a minor.

[Reinmar]

Unfortunately, I can see a significant performance drop after these changes.

This is master:

This is with the proposed changes:

The first task just got 3x longer. It's clearly visible that it contains a lot more calls.

[Reinmar]

IDK which bottleneck we're hitting right now, but I found a followup which may be worth checking if we identify the issue to be in mutation observer: ckeditor/ckeditor5#5600.

[mlewand]

We just had a discussion with @Reinmar about this issue. Here are following actions:

• widget resizer will continue to use views as proposed in this PR
• we'll hide the image toolbar during the image resize (It should be possible to hide/disable a widget toolbar ckeditor5#5964)
• we'll fix an issue where ui is forced to be re-rendered if no meaningful mutations were recorded (Performance improvement in case of empty mutations ckeditor5#5600)

More details:

The performance hit comes from the fact that now as the code use CKE5 views for DOM manipulation (does not do changes in the DOM directly), there are two View#change() requests. Each of these change requests finishes with a render event. And on render event the image inline toolbar is repositioned - this is where the performance penalty comes from.

On a side note there's also slightly related problem, where changes in our size UI are causing extra mutations. That's tracked in ckeditor/ckeditor5#5600.

[mlewand]

This PR is blocked by ckeditor/ckeditor5#5964 as it needs to utilize WidgetToolbarRepository forceDisabled() method to hide the toolbar during the resize. Otherwise there are visible performance regression.

[Reinmar]

I tested the new disable/enable API as mentioned in #113. I think you can update this PR now.

[mlewand]

Now resizer hides widget toolbar during the resize. I made a soft check on WidgetToolbarRepository plugin as it may or may not be available.

Also the actual toolbar hide logic was hooked to Resizer's begin, commit and cancel methods instead of mouse listener code directly, so that it works if resizer is operated using the API.

[Reinmar]

If you click on the resize handle, the toolbar disappears and it never reappears, even if you select another widget.

[FilipTokarski]

Toolbar works fine now.

There is a strange issue with resizing an image that is in Fancy editor (padding inside figure) and has width: not set. If you try to resize it veery slowly it suddenly jumps and shrinks just a bit. Sometimes jumps once again when releasing the mouse button. It happens only on the first attempt.
Browser: any / os: any

[Mgsy]

☝️ same in Safari, however, I didn't notice weird behaviour after releasing the mouse button.

Besides this small issue, it looks good.

[Reinmar]

Yeah, the logic for handling an image without a width is tricky and can manifest like that. We had to take some shortcuts there – basically, we estimate the real width. 

Thanks for checking this PR. I'm merging it.