Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image jumps when resizing in container that has padding #16304

Merged
merged 9 commits into from
May 7, 2024
Merged

Image jumps when resizing in container that has padding #16304

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c26cc45
Fix resize image in pagination mode
Mati365 May 2, 2024
e2886f1
Adjust doc
Mati365 May 2, 2024
ac0d359
Fix tests
Mati365 May 2, 2024
9c6da53
Fix typo
Mati365 May 2, 2024
05ac30f
Fix tests
Mati365 May 2, 2024
114f690
Adjust tests
Mati365 May 2, 2024
5d28ffa
Change resize host getter result
Mati365 May 7, 2024
dae3287
Adjust tests
Mati365 May 7, 2024
c93633b
Merge branch 'master' into ck/14698
oleq May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions packages/ckeditor5-widget/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,20 +453,35 @@ function addSelectionHandle( widgetElement: ViewContainerElement, writer: Downca
* Starting from a DOM resize host element (an element that receives dimensions as a result of resizing),
* this helper returns the width of the found ancestor element.
*
* **Note**: This helper searches up to 5 levels of ancestors only.
* * It searches up to 5 levels of ancestors only.
* It will skip search if passed element (or it's parent is contenteditable).
*
* @param domResizeHost Resize host DOM element that receives dimensions as a result of resizing.
* @returns Width of ancestor element in pixels or 0 if no ancestor with a computed width has been found.
*/
export function calculateResizeHostAncestorWidth( domResizeHost: HTMLElement ): number {
const getElementComputedWidth = ( element: HTMLElement ) => {
const { width, paddingLeft, paddingRight } = element.ownerDocument.defaultView!.getComputedStyle( element! );

return parseFloat( width ) - parseFloat( paddingLeft ) - parseFloat( paddingRight );
};

if ( isMaximumResizeHostAncestorElement( domResizeHost ) ) {
return getElementComputedWidth( domResizeHost );
}

const domResizeHostParent = domResizeHost.parentElement;

if ( !domResizeHostParent ) {
return 0;
}

// Need to use computed style as it properly excludes parent's paddings from the returned value.
let parentWidth = parseFloat( domResizeHostParent!.ownerDocument.defaultView!.getComputedStyle( domResizeHostParent! ).width );
let parentWidth = getElementComputedWidth( domResizeHostParent! );

if ( isMaximumResizeHostAncestorElement( domResizeHostParent ) ) {
Copy link
Member

@oleq oleq May 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, in my opinion, is only masking the actual root of the issue in ImageResizeHandles. Check out the current implementation at

ckeditor5/packages/ckeditor5-image/src/imageresize/imageresizehandles.ts

Line 103 in 67d2d60

getResizeHost() {
and see what the output is for various combinations of editors (decoupled, classic) and images (block, inline).

Long story short, it returns paragraphs for inline images and editing roots for block images (that exist directly in a root). I think this is wrong. According to _getResizeHost() in Resizer class

Resize host is an object that receives dimensions which are the result of resizing.

ckeditor5/packages/ckeditor5-widget/src/widgetresize/resizer.ts

Lines 413 to 422 in 67d2d60

/**
* Obtains the resize host.
*
* Resize host is an object that receives dimensions which are the result of resizing.
*/
private _getResizeHost(): HTMLElement {
const widgetWrapper = this._domResizerWrapper!.parentElement;
return this._options.getResizeHost( widgetWrapper! );
}

An editing root does not receive dimensions. <figure> does (block image) or <span> (inline image).

return parentWidth;
}

// Sometimes parent width cannot be accessed. If that happens we should go up in the elements tree
// and try to get width from next ancestor.
Expand All @@ -483,14 +498,23 @@ export function calculateResizeHostAncestorWidth( domResizeHost: HTMLElement ):
return 0;
}

parentWidth = parseFloat(
domResizeHostParent!.ownerDocument.defaultView!.getComputedStyle( checkedElement ).width
);
parentWidth = getElementComputedWidth( checkedElement );

if ( isMaximumResizeHostAncestorElement( checkedElement ) ) {
break;
}
}

return parentWidth;
}

/**
* Checks if passed HTML has `contenteditable` attribute.
*/
function isMaximumResizeHostAncestorElement( element: HTMLElement ) {
return element.getAttribute( 'contenteditable' ) === 'true' || element.classList.contains( 'ck-editor__editable' );
}

/**
* Calculates a relative width of a `domResizeHost` compared to its ancestor in percents.
*
Expand Down