t/153: Rect utility should work for collapsed DOM Ranges. Closes #153. #154
Conversation
src/dom/rect.js
Outdated
| // | ||
| // @private | ||
| // @param {ClientRect|module:utils/dom/rect~Rect|Object} source} | ||
| const setProperties = source => { |
There was a problem hiding this comment.
What about this then? I think it would only complicate the code.
src/dom/rect.js
Outdated
|
|
||
| rectProperties.forEach( p => this[ p ] = source[ p ] ); | ||
| if ( isElement( source ) ) { | ||
| setProperties( source.getBoundingClientRect() ); |
There was a problem hiding this comment.
I don't like how many times setProperties is executed in this file. Couldn't the rect be assigned to some value and later on the values be set once?
There was a problem hiding this comment.
It used to be that way. But since the new case arrived, wherewidth must be set to 0 and the output of getBoundingClientRect/getClientRects is read–only, I would need to use an intermediate object to write client rect's properties to it (in a loop, Object.assign fails) and then alter the witdth, which doesn't look pretty. So I decided this looks better.
There was a problem hiding this comment.
KK. Makes sense. I didn't now it's read only.
Suggested merge commit message (convention)
Fix: Rect utility should work for collapsed DOM Ranges. Closes ckeditor/ckeditor5#4952.