From 95d5dac2428650cfa0ced30eff0b1bb720981f37 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 14 Aug 2019 15:17:19 +0200 Subject: [PATCH] Fix: `Rect#excludeScrollbarsAndBorders` should support RTL content. Fixed incorrect output of the method. Closes #297. --- src/dom/rect.js | 23 ++++++--- tests/dom/rect.js | 119 ++++++++++++++++++++++++++++++++++++++------ tests/dom/scroll.js | 9 ++-- 3 files changed, 127 insertions(+), 24 deletions(-) diff --git a/src/dom/rect.js b/src/dom/rect.js index 3a832bb..fae9c1c 100644 --- a/src/dom/rect.js +++ b/src/dom/rect.js @@ -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; diff --git a/tests/dom/rect.js b/tests/dom/rect.js index 47ebc4d..0cee3f2 100644 --- a/tests/dom/rect.js +++ b/tests/dom/rect.js @@ -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 @@ -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 ); @@ -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, @@ -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' ); @@ -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; @@ -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, diff --git a/tests/dom/scroll.js b/tests/dom/scroll.js index 602bb33..caf83e3 100644 --- a/tests/dom/scroll.js +++ b/tests/dom/scroll.js @@ -28,7 +28,8 @@ describe( 'scrollAncestorsToShowTarget()', () => { borderTopWidth: '0px', borderRightWidth: '0px', borderBottomWidth: '0px', - borderLeftWidth: '0px' + borderLeftWidth: '0px', + direction: 'ltr' } ); stubRect( firstAncestor, { @@ -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. @@ -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.