Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #298 from ckeditor/t/297
Browse files Browse the repository at this point in the history 
Fix: `Rect#excludeScrollbarsAndBorders` should support RTL environments. Fixed incorrect output of the method. Closes #297.
  • Loading branch information
@jodator
jodator authored Sep 9, 2019
2 parents fb8fbd1 + 95d5dac commit 35f34fc
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 24 deletions.
23 changes: 17 additions & 6 deletions src/dom/rect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,23 +310,34 @@ export default class Rect {
*/
excludeScrollbarsAndBorders() {
const source = this._source;
let scrollBarWidth, scrollBarHeight;
let scrollBarWidth, scrollBarHeight, direction;

if ( isWindow( source ) ) {
scrollBarWidth = source.innerWidth - source.document.documentElement.clientWidth;
scrollBarHeight = source.innerHeight - source.document.documentElement.clientHeight;
direction = source.getComputedStyle( source.document.documentElement ).direction;
} else {
const borderWidths = getBorderWidths( this._source );

scrollBarWidth = source.offsetWidth - source.clientWidth;
scrollBarHeight = source.offsetHeight - source.clientHeight;
scrollBarWidth = source.offsetWidth - source.clientWidth - borderWidths.left - borderWidths.right;
scrollBarHeight = source.offsetHeight - source.clientHeight - borderWidths.top - borderWidths.bottom;
direction = source.ownerDocument.defaultView.getComputedStyle( source ).direction;

this.moveBy( borderWidths.left, borderWidths.top );
this.left += borderWidths.left;
this.top += borderWidths.top;
this.right -= borderWidths.right;
this.bottom -= borderWidths.bottom;
this.width = this.right - this.left;
this.height = this.bottom - this.top;
}

// Assuming LTR scrollbars. TODO: RTL.
this.width -= scrollBarWidth;
this.right -= scrollBarWidth;

if ( direction === 'ltr' ) {
this.right -= scrollBarWidth;
} else {
this.left += scrollBarWidth;
}

this.height -= scrollBarHeight;
this.bottom -= scrollBarHeight;
Expand Down
119 changes: 104 additions & 15 deletions tests/dom/rect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -792,18 +792,21 @@ describe( 'Rect', () => {
} );

describe( 'excludeScrollbarsAndBorders()', () => {
it( 'should exclude scrollbars and borders of a HTMLElement', () => {
it( 'should exclude scrollbars and borders of a HTMLElement (dir="ltr")', () => {
const element = document.createElement( 'div' );

sinon.stub( element, 'getBoundingClientRect' ).returns( geometry );
sinon.stub( window, 'getComputedStyle' ).returns( {
borderTopWidth: '5px',
borderRightWidth: '10px',
borderLeftWidth: '5px',
borderBottomWidth: '10px'
} );
sinon.stub( window, 'getComputedStyle' )
.withArgs( element )
.returns( {
borderTopWidth: '5px',
borderRightWidth: '10px',
borderLeftWidth: '5px',
borderBottomWidth: '10px',
direction: 'ltr'
} );

// Simulate 5px srollbars.
// Simulate 5px scrollbars.
Object.defineProperties( element, {
offsetWidth: {
value: 20
Expand All @@ -812,24 +815,65 @@ describe( 'Rect', () => {
value: 20
},
clientWidth: {
value: 10
value: 0
},
clientHeight: {
value: 10
value: 0
}
} );

assertRect( new Rect( element ).excludeScrollbarsAndBorders(), {
top: 15,
right: 35,
bottom: 25,
right: 25,
bottom: 15,
left: 25,
width: 10,
height: 10
width: 0,
height: 0
} );
} );

it( 'should exclude scrollbars and borders of a HTMLElement (dir="rtl")', () => {
const element = document.createElement( 'div' );

element.setAttribute( 'dir', 'rtl' );
sinon.stub( element, 'getBoundingClientRect' ).returns( geometry );
sinon.stub( window, 'getComputedStyle' )
.withArgs( element )
.returns( {
borderTopWidth: '5px',
borderRightWidth: '10px',
borderLeftWidth: '5px',
borderBottomWidth: '10px',
direction: 'rtl'
} );

// Simulate 5px scrollbars.
Object.defineProperties( element, {
offsetWidth: {
value: 20
},
offsetHeight: {
value: 20
},
clientWidth: {
value: 0
},
clientHeight: {
value: 0
}
} );

assertRect( new Rect( element ).excludeScrollbarsAndBorders(), {
top: 15,
right: 30,
bottom: 15,
left: 30,
width: 0,
height: 0
} );
} );

it( 'should exclude scrollbars from viewport\'s rect', () => {
it( 'should exclude scrollbars from viewport\'s rect (dir="ltr")', () => {
sinon.stub( window, 'innerWidth' ).value( 1000 );
sinon.stub( window, 'innerHeight' ).value( 500 );
sinon.stub( window, 'scrollX' ).value( 100 );
Expand All @@ -840,6 +884,12 @@ describe( 'Rect', () => {
clientHeight: 490
} );

sinon.stub( window, 'getComputedStyle' )
.withArgs( document.documentElement )
.returns( {
direction: 'ltr'
} );

assertRect( new Rect( window ).excludeScrollbarsAndBorders(), {
top: 0,
right: 990,
Expand All @@ -850,6 +900,33 @@ describe( 'Rect', () => {
} );
} );

it( 'should exclude scrollbars from viewport\'s rect (dir="rtl")', () => {
sinon.stub( window, 'innerWidth' ).value( 1000 );
sinon.stub( window, 'innerHeight' ).value( 500 );
sinon.stub( window, 'scrollX' ).value( 100 );
sinon.stub( window, 'scrollY' ).value( 200 );

sinon.stub( document, 'documentElement' ).value( {
clientWidth: 990,
clientHeight: 490
} );

sinon.stub( window, 'getComputedStyle' )
.withArgs( document.documentElement )
.returns( {
direction: 'rtl'
} );

assertRect( new Rect( window ).excludeScrollbarsAndBorders(), {
top: 0,
right: 1000,
bottom: 490,
left: 10,
width: 990,
height: 490
} );
} );

it( 'should work for a window in an iframe', done => {
const iframe = document.createElement( 'iframe' );

Expand All @@ -864,6 +941,12 @@ describe( 'Rect', () => {
clientHeight: 490
} );

sinon.stub( window, 'getComputedStyle' )
.withArgs( document.documentElement )
.returns( {
direction: 'ltr'
} );

iframe.addEventListener( 'load', () => {
const iframeWindow = iframe.contentWindow;

Expand All @@ -877,6 +960,12 @@ describe( 'Rect', () => {
clientHeight: 230
} );

sinon.stub( iframeWindow, 'getComputedStyle' )
.withArgs( iframeWindow.document.documentElement )
.returns( {
direction: 'ltr'
} );

assertRect( new Rect( iframeWindow ).excludeScrollbarsAndBorders(), {
top: 0,
right: 480,
Expand Down
9 changes: 6 additions & 3 deletions tests/dom/scroll.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ describe( 'scrollAncestorsToShowTarget()', () => {
borderTopWidth: '0px',
borderRightWidth: '0px',
borderBottomWidth: '0px',
borderLeftWidth: '0px'
borderLeftWidth: '0px',
direction: 'ltr'
} );

stubRect( firstAncestor, {
Expand Down Expand Up @@ -169,7 +170,8 @@ describe( 'scrollViewportToShowTarget()', () => {
borderTopWidth: '0px',
borderRightWidth: '0px',
borderBottomWidth: '0px',
borderLeftWidth: '0px'
borderLeftWidth: '0px',
direction: 'ltr'
} );

// Assuming 20px v- and h-scrollbars here.
Expand Down Expand Up @@ -228,7 +230,8 @@ describe( 'scrollViewportToShowTarget()', () => {
borderTopWidth: '0px',
borderRightWidth: '0px',
borderBottomWidth: '0px',
borderLeftWidth: '0px'
borderLeftWidth: '0px',
direction: 'ltr'
} );

// Assuming 20px v- and h-scrollbars here.
Expand Down

0 comments on commit 35f34fc

Please sign in to comment.