ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Fix (widget): Don't show image resizer handle when resizer is not enabled (commentsOnly mode)

Open kbinieda opened this issue 3 years ago • 1 comments

Suggested merge commit message (convention)

Fix (widget): Don't show image resizer handle when resizer is not enabled (commentsOnly mode). Closes #11891.


Additional information

Main issue was, probably race condition between change of visibleResizer attribute(https://github.com/ckeditor/ckeditor5/blob/e51d6c564261889433c04fb8b2bcaa24a38bf0a5/packages/ckeditor5-widget/src/widgetresize.js#L90) and change of isEnabled attribute (https://github.com/ckeditor/ckeditor5/blob/e51d6c564261889433c04fb8b2bcaa24a38bf0a5/packages/ckeditor5-widget/src/widgetresize/resizer.js#L115).

kbinieda avatar Sep 09 '22 09:09 kbinieda

I see that you requested a review but only changed the place where the fix is applied.

I wrote a response regarding refactoring WidgetResize and Resizer and how they communicate. My goal was to guide you what seems to me incorrect in current implementation of these classes. I would like you to implement this refinement, as I described (unless, of course, there are some problems or some better ideas).

I short summary, Resizer should have isEnabled and isSelected and maybe isVisible (but this one is not set directly but bound to the other too). WidgetResizer should have selectedResizer (rename of visibleResizer) and set Resizer#isSelected. Also some other fixes along way. Please read the full post above.

scofalik avatar Sep 16 '22 06:09 scofalik