From 11a6e935519bedf7effe0f1acf06bda0b3b8ad27 Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 6 Aug 2019 22:10:23 +0200 Subject: [PATCH 01/11] Typewriter XP --- packages/block-editor/README.md | 5 + .../src/components/block-list/index.js | 6 +- packages/block-editor/src/components/index.js | 1 + .../src/components/typewriter/index.js | 155 ++++++++++++++++++ .../src/components/writing-flow/index.js | 2 +- .../src/components/writing-flow/style.scss | 2 - packages/e2e-tests/specs/writing-flow.test.js | 42 +++++ .../src/components/visual-editor/index.js | 19 ++- .../src/components/visual-editor/style.scss | 8 +- 9 files changed, 223 insertions(+), 17 deletions(-) create mode 100644 packages/block-editor/src/components/typewriter/index.js diff --git a/packages/block-editor/README.md b/packages/block-editor/README.md index 1d230d45fc3a23..c988010de4e624 100644 --- a/packages/block-editor/README.md +++ b/packages/block-editor/README.md @@ -420,6 +420,11 @@ _Returns_ - `Array`: converted rules. +# **Typewriter** + +Ensures that the text selection keeps the same vertical distance from the +viewport during keyboard events within this component. + # **URLInput** _Related_ diff --git a/packages/block-editor/src/components/block-list/index.js b/packages/block-editor/src/components/block-list/index.js index 623840c3492ee3..e6fa2d7b8cb605 100644 --- a/packages/block-editor/src/components/block-list/index.js +++ b/packages/block-editor/src/components/block-list/index.js @@ -263,6 +263,7 @@ export default compose( [ getMultiSelectedBlockClientIds, hasMultiSelection, getGlobalBlockCount, + isTyping, } = select( 'core/block-editor' ); const { rootClientId } = ownProps; @@ -276,7 +277,10 @@ export default compose( [ selectedBlockClientId: getSelectedBlockClientId(), multiSelectedBlockClientIds: getMultiSelectedBlockClientIds(), hasMultiSelection: hasMultiSelection(), - enableAnimation: getGlobalBlockCount() <= BLOCK_ANIMATION_THRESHOLD, + enableAnimation: ( + ! isTyping() && + getGlobalBlockCount() <= BLOCK_ANIMATION_THRESHOLD + ), }; } ), withDispatch( ( dispatch ) => { diff --git a/packages/block-editor/src/components/index.js b/packages/block-editor/src/components/index.js index 9ad37a8929b433..729fe3fd222295 100644 --- a/packages/block-editor/src/components/index.js +++ b/packages/block-editor/src/components/index.js @@ -59,6 +59,7 @@ export { default as NavigableToolbar } from './navigable-toolbar'; export { default as ObserveTyping } from './observe-typing'; export { default as PreserveScrollInReorder } from './preserve-scroll-in-reorder'; export { default as SkipToSelectedBlock } from './skip-to-selected-block'; +export { default as Typewriter } from './typewriter'; export { default as Warning } from './warning'; export { default as WritingFlow } from './writing-flow'; diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js new file mode 100644 index 00000000000000..6604eadc976296 --- /dev/null +++ b/packages/block-editor/src/components/typewriter/index.js @@ -0,0 +1,155 @@ +/** + * External dependencies + */ +import { debounce } from 'lodash'; + +/** + * WordPress dependencies + */ +import { Component, createRef } from '@wordpress/element'; +import { computeCaretRect, getScrollContainer } from '@wordpress/dom'; +import { withSelect } from '@wordpress/data'; +import { compose } from '@wordpress/compose'; + +class Typewriter extends Component { + constructor() { + super( ...arguments ); + + this.ref = createRef(); + this.onKeyDown = this.onKeyDown.bind( this ); + this.onMouseDown = this.onMouseDown.bind( this ); + this.onTouchStart = this.onTouchStart.bind( this ); + this.maintainCaretPositionOnSelectionChange = this.maintainCaretPositionOnSelectionChange.bind( this ); + this.computeCaretRectOnSelectionChange = this.computeCaretRectOnSelectionChange.bind( this ); + this.maintainCaretPosition = this.maintainCaretPosition.bind( this ); + this.computeCaretRect = this.computeCaretRect.bind( this ); + this.debouncedComputeCaretRect = debounce( this.computeCaretRect, 100 ); + this.isSelectionEligibleForScroll = this.isSelectionEligibleForScroll.bind( this ); + } + + componentDidMount() { + window.addEventListener( 'scroll', this.debouncedComputeCaretRect, true ); + window.addEventListener( 'resize', this.debouncedComputeCaretRect, true ); + } + + componentWillUnmount() { + window.removeEventListener( 'scroll', this.debouncedComputeCaretRect, true ); + window.removeEventListener( 'resize', this.debouncedComputeCaretRect, true ); + document.removeEventListener( 'selectionchange', this.maintainCaretPositionOnSelectionChange ); + document.removeEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); + window.cancelAnimationFrame( this.rafId ); + this.debouncedComputeCaretRect.cancel(); + } + + computeCaretRect() { + if ( this.isSelectionEligibleForScroll() ) { + this.caretRect = computeCaretRect(); + } + } + + maintainCaretPositionOnSelectionChange() { + document.removeEventListener( 'selectionchange', this.maintainCaretPositionOnSelectionChange ); + this.maintainCaretPosition(); + } + + computeCaretRectOnSelectionChange() { + document.removeEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); + this.computeCaretRect(); + } + + isSelectionEligibleForScroll() { + return ( + // There should be one and only one block selected. + this.props.selectedBlockClientId && + // The component must contain the selection. + this.ref.current.contains( document.activeElement ) && + // The active element must be contenteditable. + document.activeElement.isContentEditable + ); + } + + maintainCaretPosition() { + if ( ! this.caretRect ) { + this.computeCaretRect(); + return; + } + + if ( ! this.isSelectionEligibleForScroll() ) { + return; + } + + const currentCaretRect = computeCaretRect(); + + if ( ! currentCaretRect ) { + return; + } + + const diff = currentCaretRect.y - this.caretRect.y; + + if ( diff === 0 ) { + return; + } + + const scrollContainer = getScrollContainer( this.ref.current ); + + // The page must be scrollable. + if ( ! scrollContainer ) { + return; + } + + // The scroll container may be different depending on the viewport + // width. + if ( scrollContainer === document.body ) { + window.scrollBy( 0, diff ); + } else { + scrollContainer.scrollTop += diff; + } + } + + onMouseDown() { + document.addEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); + } + + onTouchStart() { + document.addEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); + } + + onKeyDown() { + // Ensure the any remaining request is cancelled. + window.cancelAnimationFrame( this.rafId ); + // Use an animation frame for a smooth result. + this.rafId = window.requestAnimationFrame( this.maintainCaretPosition ); + // In rare cases, the selection is not updated before the animation + // frame. This selection change listener is a back up. + document.addEventListener( 'selectionchange', this.maintainCaretPositionOnSelectionChange ); + } + + render() { + // Disable reason: Wrapper itself is non-interactive, but must capture + // bubbling events from children to determine focus transition intents. + /* eslint-disable jsx-a11y/no-static-element-interactions */ + return ( +
+ { this.props.children } +
+ ); + /* eslint-enable jsx-a11y/no-static-element-interactions */ + } +} + +/** + * Ensures that the text selection keeps the same vertical distance from the + * viewport during keyboard events within this component. + */ +export default compose( [ + withSelect( ( select ) => { + const { getSelectedBlockClientId } = select( 'core/block-editor' ); + return { selectedBlockClientId: getSelectedBlockClientId() }; + } ), +] )( Typewriter ); diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index cbc531ef754a85..54e1a7ee77fe08 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -404,7 +404,7 @@ class WritingFlow extends Component { aria-hidden tabIndex={ -1 } onClick={ this.focusLastTextField } - className="wp-block editor-writing-flow__click-redirect block-editor-writing-flow__click-redirect" + className="editor-writing-flow__click-redirect block-editor-writing-flow__click-redirect" /> ); diff --git a/packages/block-editor/src/components/writing-flow/style.scss b/packages/block-editor/src/components/writing-flow/style.scss index e1ff5e860ad149..422b378f2e8e2e 100644 --- a/packages/block-editor/src/components/writing-flow/style.scss +++ b/packages/block-editor/src/components/writing-flow/style.scss @@ -1,10 +1,8 @@ .block-editor-writing-flow { - height: 100%; display: flex; flex-direction: column; } .block-editor-writing-flow__click-redirect { - flex-basis: 100%; cursor: text; } diff --git a/packages/e2e-tests/specs/writing-flow.test.js b/packages/e2e-tests/specs/writing-flow.test.js index f0683ff51e0f09..395164fe7a892f 100644 --- a/packages/e2e-tests/specs/writing-flow.test.js +++ b/packages/e2e-tests/specs/writing-flow.test.js @@ -422,4 +422,46 @@ describe( 'Writing Flow', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + it( 'should maintain caret position', async () => { + // Create first block. + await page.keyboard.press( 'Enter' ); + + // Create blocks until the page is scrollable. + while ( await page.evaluate( () => + ! wp.dom.getScrollContainer( document.activeElement ) + ) ) { + await page.keyboard.press( 'Enter' ); + } + + const initialPosition = await page.evaluate( () => + wp.dom.computeCaretRect().y + ); + + await page.keyboard.press( 'Enter' ); + + expect( await page.evaluate( () => + wp.dom.computeCaretRect().y + ) ).toBe( initialPosition ); + + // Type until the text wraps. + while ( await page.evaluate( () => + document.activeElement.clientHeight <= + parseInt( getComputedStyle( document.activeElement ).lineHeight, 10 ) + ) ) { + await page.keyboard.type( 'a' ); + } + + expect( await page.evaluate( () => + wp.dom.computeCaretRect().y + ) ).toBe( initialPosition ); + + // Pressing backspace will reposition the caret to the previous line. + // Scroll position should be adjusted again. + await page.keyboard.press( 'Backspace' ); + + expect( await page.evaluate( () => + wp.dom.computeCaretRect().y + ) ).toBe( initialPosition ); + } ); } ); diff --git a/packages/edit-post/src/components/visual-editor/index.js b/packages/edit-post/src/components/visual-editor/index.js index 8287f5c27396d5..37d3749f0f4983 100644 --- a/packages/edit-post/src/components/visual-editor/index.js +++ b/packages/edit-post/src/components/visual-editor/index.js @@ -7,6 +7,7 @@ import { } from '@wordpress/editor'; import { WritingFlow, + Typewriter, ObserveTyping, BlockList, CopyHandler, @@ -27,14 +28,16 @@ function VisualEditor() { - - - - - - - - + + + + + + + + + + <__experimentalBlockSettingsMenuFirstItem> { ( { onClose } ) => } diff --git a/packages/edit-post/src/components/visual-editor/style.scss b/packages/edit-post/src/components/visual-editor/style.scss index c02a1c7dbcacc8..fb3bf165b4a326 100644 --- a/packages/edit-post/src/components/visual-editor/style.scss +++ b/packages/edit-post/src/components/visual-editor/style.scss @@ -1,6 +1,6 @@ .edit-post-visual-editor { position: relative; - padding: 50px 0; + padding-top: 50px; & .components-button { font-family: $default-font; @@ -8,11 +8,9 @@ } .edit-post-visual-editor .block-editor-writing-flow__click-redirect { - // Collapse to minimum height of 50px, to fully occupy editor bottom pad. - height: 50px; + // Allow the page to be scrolled with the last block in the middle. + height: 50vh; width: 100%; - // Offset for: Visual editor padding, block (default appender) margin. - margin: #{ -1 * $block-spacing } auto -50px; } // The base width of blocks From 36ee38f29f4e7023927c2598610216bc08e151a4 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 19 Aug 2019 16:40:49 +0200 Subject: [PATCH 02/11] Address feedback --- .../src/components/typewriter/index.js | 109 ++++++++++----- packages/e2e-tests/specs/typewriter.test.js | 126 ++++++++++++++++++ packages/e2e-tests/specs/writing-flow.test.js | 42 ------ 3 files changed, 202 insertions(+), 75 deletions(-) create mode 100644 packages/e2e-tests/specs/typewriter.test.js diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js index 6604eadc976296..7acab6d255faa3 100644 --- a/packages/block-editor/src/components/typewriter/index.js +++ b/packages/block-editor/src/components/typewriter/index.js @@ -9,7 +9,10 @@ import { debounce } from 'lodash'; import { Component, createRef } from '@wordpress/element'; import { computeCaretRect, getScrollContainer } from '@wordpress/dom'; import { withSelect } from '@wordpress/data'; -import { compose } from '@wordpress/compose'; +import { UP, DOWN, LEFT, RIGHT } from '@wordpress/keycodes'; + +const isIE = window.navigator.userAgent.indexOf( 'Trident' ) !== -1; +const arrowKeyCodes = new Set( [ UP, DOWN, LEFT, RIGHT ] ); class Typewriter extends Component { constructor() { @@ -17,9 +20,7 @@ class Typewriter extends Component { this.ref = createRef(); this.onKeyDown = this.onKeyDown.bind( this ); - this.onMouseDown = this.onMouseDown.bind( this ); - this.onTouchStart = this.onTouchStart.bind( this ); - this.maintainCaretPositionOnSelectionChange = this.maintainCaretPositionOnSelectionChange.bind( this ); + this.addSelectionChangeListener = this.addSelectionChangeListener.bind( this ); this.computeCaretRectOnSelectionChange = this.computeCaretRectOnSelectionChange.bind( this ); this.maintainCaretPosition = this.maintainCaretPosition.bind( this ); this.computeCaretRect = this.computeCaretRect.bind( this ); @@ -28,6 +29,8 @@ class Typewriter extends Component { } componentDidMount() { + // When the user scrolls or resizes, the scroll position should be + // reset. window.addEventListener( 'scroll', this.debouncedComputeCaretRect, true ); window.addEventListener( 'resize', this.debouncedComputeCaretRect, true ); } @@ -35,45 +38,65 @@ class Typewriter extends Component { componentWillUnmount() { window.removeEventListener( 'scroll', this.debouncedComputeCaretRect, true ); window.removeEventListener( 'resize', this.debouncedComputeCaretRect, true ); - document.removeEventListener( 'selectionchange', this.maintainCaretPositionOnSelectionChange ); document.removeEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); window.cancelAnimationFrame( this.rafId ); this.debouncedComputeCaretRect.cancel(); } + /** + * Resets the scroll position to be maintained. + */ computeCaretRect() { if ( this.isSelectionEligibleForScroll() ) { this.caretRect = computeCaretRect(); } } - maintainCaretPositionOnSelectionChange() { - document.removeEventListener( 'selectionchange', this.maintainCaretPositionOnSelectionChange ); - this.maintainCaretPosition(); - } - + /** + * Resets the scroll position to be maintained during a `selectionchange` + * event. Also removes the listener, so it acts as a one-time listener. + */ computeCaretRectOnSelectionChange() { document.removeEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); this.computeCaretRect(); } + /** + * Checks if the current situation is elegible for scroll: + * - There should be one and only one block selected. + * - The component must contain the selection. + * - The active element must be contenteditable. + */ isSelectionEligibleForScroll() { return ( - // There should be one and only one block selected. this.props.selectedBlockClientId && - // The component must contain the selection. this.ref.current.contains( document.activeElement ) && - // The active element must be contenteditable. document.activeElement.isContentEditable ); } - maintainCaretPosition() { + /** + * Maintains the scroll position after a selection change caused by a + * keyboard event. + * + * @param {SyntheticEvent} event Synthetic keyboard event. + */ + maintainCaretPosition( { keyCode } ) { + // If for some reason there is no position set to be scrolled to, let + // this be the position to be scrolled to in the future. if ( ! this.caretRect ) { this.computeCaretRect(); return; } + // Even though enabling the typewriter effect for arrow keys results in + // a pleasant experience, it may not be the case for everyone, so, for + // now, let's disable it. + if ( arrowKeyCodes.has( keyCode ) ) { + this.computeCaretRect(); + return; + } + if ( ! this.isSelectionEligibleForScroll() ) { return; } @@ -97,34 +120,56 @@ class Typewriter extends Component { return; } + const editableNodes = this.ref.current.querySelectorAll( '[contenteditable="true"]' ); + const lastEditableNode = editableNodes[ editableNodes.length - 1 ]; + const isLastEditableNode = lastEditableNode === document.activeElement; + // The scroll container may be different depending on the viewport // width. + // Nested condition: if the scroll position is at the start and the + // active editable element is the last one, do not scroll the page. + // The typewriter effect should not kick in until an empty page has been + // filled or the user scrolls intentionally down. if ( scrollContainer === document.body ) { + if ( isLastEditableNode && window.scrollY === 0 ) { + return; + } + window.scrollBy( 0, diff ); } else { + if ( isLastEditableNode && scrollContainer.scrollTop === 0 ) { + return; + } + scrollContainer.scrollTop += diff; } } - onMouseDown() { - document.addEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); - } - - onTouchStart() { + /** + * Adds a `selectionchange` listener to reset the scroll position to be + * maintained. + */ + addSelectionChangeListener() { document.addEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); } - onKeyDown() { + onKeyDown( event ) { + event.persist(); // Ensure the any remaining request is cancelled. window.cancelAnimationFrame( this.rafId ); // Use an animation frame for a smooth result. - this.rafId = window.requestAnimationFrame( this.maintainCaretPosition ); - // In rare cases, the selection is not updated before the animation - // frame. This selection change listener is a back up. - document.addEventListener( 'selectionchange', this.maintainCaretPositionOnSelectionChange ); + this.rafId = window.requestAnimationFrame( () => + this.maintainCaretPosition( event ) + ); } render() { + // There are some issues with Internet Explorer, which are probably not + // worth spending time on. Let's disable it. + if ( isIE ) { + return null; + } + // Disable reason: Wrapper itself is non-interactive, but must capture // bubbling events from children to determine focus transition intents. /* eslint-disable jsx-a11y/no-static-element-interactions */ @@ -132,9 +177,9 @@ class Typewriter extends Component {
{ this.props.children }
@@ -147,9 +192,7 @@ class Typewriter extends Component { * Ensures that the text selection keeps the same vertical distance from the * viewport during keyboard events within this component. */ -export default compose( [ - withSelect( ( select ) => { - const { getSelectedBlockClientId } = select( 'core/block-editor' ); - return { selectedBlockClientId: getSelectedBlockClientId() }; - } ), -] )( Typewriter ); +export default withSelect( ( select ) => { + const { getSelectedBlockClientId } = select( 'core/block-editor' ); + return { selectedBlockClientId: getSelectedBlockClientId() }; +} )( Typewriter ); diff --git a/packages/e2e-tests/specs/typewriter.test.js b/packages/e2e-tests/specs/typewriter.test.js new file mode 100644 index 00000000000000..3cde8663c4d6c3 --- /dev/null +++ b/packages/e2e-tests/specs/typewriter.test.js @@ -0,0 +1,126 @@ +/** + * WordPress dependencies + */ +import { createNewPost } from '@wordpress/e2e-test-utils'; + +describe( 'TypeWriter', () => { + beforeEach( async () => { + await createNewPost(); + } ); + + const expectScrollPositionToBe = async ( expectedScrollPosition ) => { + const currentScrollPosition = await page.evaluate( () => + wp.dom.computeCaretRect().y + ); + const diff = Math.abs( currentScrollPosition - expectedScrollPosition ); + + // Allow the scroll position to be 1px off. + expect( diff ).toBeLessThanOrEqual( 1 ); + }; + + it( 'should maintain caret position', async () => { + // Create first block. + await page.keyboard.press( 'Enter' ); + + const initialPosition = await page.evaluate( () => + wp.dom.computeCaretRect().y + ); + + // The page shouldn't be scrolled when it's being filled. + await page.keyboard.press( 'Enter' ); + + expect( await page.evaluate( () => + wp.dom.computeCaretRect().y + ) ).toBeGreaterThan( initialPosition ); + + // Create blocks until the browser scroll the page. + while ( await page.evaluate( () => + wp.dom.getScrollContainer( document.activeElement ).scrollTop === 0 + ) ) { + await page.keyboard.press( 'Enter' ); + } + + // Debounce time for the scroll event listener. + // eslint-disable-next-line no-restricted-syntax + await page.waitFor( 100 ); + + const newPosition = await page.evaluate( () => + wp.dom.computeCaretRect().y + ); + + // Now the scroll position should be maintained. + await page.keyboard.press( 'Enter' ); + + await expectScrollPositionToBe( newPosition ); + + // Type until the text wraps. + while ( await page.evaluate( () => + document.activeElement.clientHeight <= + parseInt( getComputedStyle( document.activeElement ).lineHeight, 10 ) + ) ) { + await page.keyboard.type( 'a' ); + } + + await expectScrollPositionToBe( newPosition ); + + // Pressing backspace will reposition the caret to the previous line. + // Scroll position should be adjusted again. + await page.keyboard.press( 'Backspace' ); + + await expectScrollPositionToBe( newPosition ); + + // Should reset scroll position to maintain. + await page.keyboard.press( 'ArrowUp' ); + + const positionAfterArrowUp = await page.evaluate( () => + wp.dom.computeCaretRect().y + ); + + expect( positionAfterArrowUp ).toBeLessThan( newPosition ); + + // Should be scrolled to new position. + await page.keyboard.press( 'Enter' ); + + await expectScrollPositionToBe( positionAfterArrowUp ); + } ); + + it( 'should maintain caret position after scroll', async () => { + // Create first block. + await page.keyboard.press( 'Enter' ); + + await page.evaluate( () => + wp.dom.getScrollContainer( document.activeElement ).scrollTop = 1 + ); + + // Debounce time for the scroll event listener. + // eslint-disable-next-line no-restricted-syntax + await page.waitFor( 100 ); + + const initialPosition = await page.evaluate( () => + wp.dom.computeCaretRect().y + ); + + // Should maintain scroll position. + await page.keyboard.press( 'Enter' ); + + await expectScrollPositionToBe( initialPosition ); + } ); + + it( 'should maintain caret position after leaving last editable', async () => { + // Create first block. + await page.keyboard.press( 'Enter' ); + // Create second block. + await page.keyboard.press( 'Enter' ); + // Move to first block. + await page.keyboard.press( 'ArrowUp' ); + + const initialPosition = await page.evaluate( () => + wp.dom.computeCaretRect().y + ); + + // Should maintain scroll position. + await page.keyboard.press( 'Enter' ); + + await expectScrollPositionToBe( initialPosition ); + } ); +} ); diff --git a/packages/e2e-tests/specs/writing-flow.test.js b/packages/e2e-tests/specs/writing-flow.test.js index 395164fe7a892f..f0683ff51e0f09 100644 --- a/packages/e2e-tests/specs/writing-flow.test.js +++ b/packages/e2e-tests/specs/writing-flow.test.js @@ -422,46 +422,4 @@ describe( 'Writing Flow', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); - - it( 'should maintain caret position', async () => { - // Create first block. - await page.keyboard.press( 'Enter' ); - - // Create blocks until the page is scrollable. - while ( await page.evaluate( () => - ! wp.dom.getScrollContainer( document.activeElement ) - ) ) { - await page.keyboard.press( 'Enter' ); - } - - const initialPosition = await page.evaluate( () => - wp.dom.computeCaretRect().y - ); - - await page.keyboard.press( 'Enter' ); - - expect( await page.evaluate( () => - wp.dom.computeCaretRect().y - ) ).toBe( initialPosition ); - - // Type until the text wraps. - while ( await page.evaluate( () => - document.activeElement.clientHeight <= - parseInt( getComputedStyle( document.activeElement ).lineHeight, 10 ) - ) ) { - await page.keyboard.type( 'a' ); - } - - expect( await page.evaluate( () => - wp.dom.computeCaretRect().y - ) ).toBe( initialPosition ); - - // Pressing backspace will reposition the caret to the previous line. - // Scroll position should be adjusted again. - await page.keyboard.press( 'Backspace' ); - - expect( await page.evaluate( () => - wp.dom.computeCaretRect().y - ) ).toBe( initialPosition ); - } ); } ); From 668de00ecdf71d5fdaa790396740141d765d4e62 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 19 Aug 2019 18:17:44 +0200 Subject: [PATCH 03/11] Do not maintain scroll position when it's out of view --- .../src/components/typewriter/index.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js index 7acab6d255faa3..096c57bfc75a9a 100644 --- a/packages/block-editor/src/components/typewriter/index.js +++ b/packages/block-editor/src/components/typewriter/index.js @@ -135,12 +135,33 @@ class Typewriter extends Component { return; } + // Abort if the target scroll position would scroll the caret out of + // view. + if ( + this.caretRect.y + this.caretRect.height > window.innerHeight || + this.caretRect.y < 0 + ) { + return; + } + window.scrollBy( 0, diff ); } else { if ( isLastEditableNode && scrollContainer.scrollTop === 0 ) { return; } + const scrollContainerRect = scrollContainer.getBoundingClientRect(); + + // Abort if the target scroll position would scroll the caret out of + // view. + if ( + this.caretRect.y + this.caretRect.height > + scrollContainerRect.y + scrollContainer.clientHeight || + this.caretRect.y < scrollContainerRect.y + ) { + return; + } + scrollContainer.scrollTop += diff; } } From ae1b9308bc88ee78c6071bddc15601228bc5eb54 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 19 Aug 2019 18:29:31 +0200 Subject: [PATCH 04/11] Reset scroll position when aborting --- packages/block-editor/src/components/typewriter/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js index 096c57bfc75a9a..a7eb5679508ca8 100644 --- a/packages/block-editor/src/components/typewriter/index.js +++ b/packages/block-editor/src/components/typewriter/index.js @@ -132,6 +132,7 @@ class Typewriter extends Component { // filled or the user scrolls intentionally down. if ( scrollContainer === document.body ) { if ( isLastEditableNode && window.scrollY === 0 ) { + this.caretRect = currentCaretRect; return; } @@ -141,12 +142,14 @@ class Typewriter extends Component { this.caretRect.y + this.caretRect.height > window.innerHeight || this.caretRect.y < 0 ) { + this.caretRect = currentCaretRect; return; } window.scrollBy( 0, diff ); } else { if ( isLastEditableNode && scrollContainer.scrollTop === 0 ) { + this.caretRect = currentCaretRect; return; } @@ -159,6 +162,7 @@ class Typewriter extends Component { scrollContainerRect.y + scrollContainer.clientHeight || this.caretRect.y < scrollContainerRect.y ) { + this.caretRect = currentCaretRect; return; } From 4ad879faa80b75ef587ea1c63905874a35f2260c Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 19 Aug 2019 18:45:35 +0200 Subject: [PATCH 05/11] Set initial trigger % --- .../src/components/typewriter/index.js | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js index a7eb5679508ca8..8c70cacf3f5165 100644 --- a/packages/block-editor/src/components/typewriter/index.js +++ b/packages/block-editor/src/components/typewriter/index.js @@ -13,6 +13,7 @@ import { UP, DOWN, LEFT, RIGHT } from '@wordpress/keycodes'; const isIE = window.navigator.userAgent.indexOf( 'Trident' ) !== -1; const arrowKeyCodes = new Set( [ UP, DOWN, LEFT, RIGHT ] ); +const initialTriggerPercentage = 0.75; class Typewriter extends Component { constructor() { @@ -123,6 +124,11 @@ class Typewriter extends Component { const editableNodes = this.ref.current.querySelectorAll( '[contenteditable="true"]' ); const lastEditableNode = editableNodes[ editableNodes.length - 1 ]; const isLastEditableNode = lastEditableNode === document.activeElement; + const scrollContainerRect = scrollContainer.getBoundingClientRect(); + const relativeScrollPosition = ( + ( this.caretRect.y - scrollContainerRect.y ) / + ( window.innerHeight - scrollContainerRect.y ) + ); // The scroll container may be different depending on the viewport // width. @@ -131,7 +137,11 @@ class Typewriter extends Component { // The typewriter effect should not kick in until an empty page has been // filled or the user scrolls intentionally down. if ( scrollContainer === document.body ) { - if ( isLastEditableNode && window.scrollY === 0 ) { + if ( + isLastEditableNode && + window.scrollY === 0 && + relativeScrollPosition < initialTriggerPercentage + ) { this.caretRect = currentCaretRect; return; } @@ -148,13 +158,15 @@ class Typewriter extends Component { window.scrollBy( 0, diff ); } else { - if ( isLastEditableNode && scrollContainer.scrollTop === 0 ) { + if ( + isLastEditableNode && + scrollContainer.scrollTop === 0 && + relativeScrollPosition < initialTriggerPercentage + ) { this.caretRect = currentCaretRect; return; } - const scrollContainerRect = scrollContainer.getBoundingClientRect(); - // Abort if the target scroll position would scroll the caret out of // view. if ( From a2242ac2d3accb2421fbdd6cb79e18dcc2a473d7 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 19 Aug 2019 23:31:04 +0200 Subject: [PATCH 06/11] Add test for viewport --- packages/e2e-tests/specs/typewriter.test.js | 124 ++++++++++++++------ 1 file changed, 91 insertions(+), 33 deletions(-) diff --git a/packages/e2e-tests/specs/typewriter.test.js b/packages/e2e-tests/specs/typewriter.test.js index 3cde8663c4d6c3..dd386ee7f9e8f7 100644 --- a/packages/e2e-tests/specs/typewriter.test.js +++ b/packages/e2e-tests/specs/typewriter.test.js @@ -8,32 +8,27 @@ describe( 'TypeWriter', () => { await createNewPost(); } ); - const expectScrollPositionToBe = async ( expectedScrollPosition ) => { - const currentScrollPosition = await page.evaluate( () => - wp.dom.computeCaretRect().y - ); - const diff = Math.abs( currentScrollPosition - expectedScrollPosition ); + const getCaretPosition = async () => + await page.evaluate( () => wp.dom.computeCaretRect().y ); + + // Allow the scroll position to be 1px off. + const BUFFER = 1; - // Allow the scroll position to be 1px off. - expect( diff ).toBeLessThanOrEqual( 1 ); - }; + const getDiff = async ( caretPosition ) => + Math.abs( await getCaretPosition() - caretPosition ); it( 'should maintain caret position', async () => { // Create first block. await page.keyboard.press( 'Enter' ); - const initialPosition = await page.evaluate( () => - wp.dom.computeCaretRect().y - ); + const initialPosition = await getCaretPosition(); // The page shouldn't be scrolled when it's being filled. await page.keyboard.press( 'Enter' ); - expect( await page.evaluate( () => - wp.dom.computeCaretRect().y - ) ).toBeGreaterThan( initialPosition ); + expect( await getCaretPosition() ).toBeGreaterThan( initialPosition ); - // Create blocks until the browser scroll the page. + // Create blocks until the the typewriter effect kicks in. while ( await page.evaluate( () => wp.dom.getScrollContainer( document.activeElement ).scrollTop === 0 ) ) { @@ -44,14 +39,12 @@ describe( 'TypeWriter', () => { // eslint-disable-next-line no-restricted-syntax await page.waitFor( 100 ); - const newPosition = await page.evaluate( () => - wp.dom.computeCaretRect().y - ); + const newPosition = await getCaretPosition(); // Now the scroll position should be maintained. await page.keyboard.press( 'Enter' ); - await expectScrollPositionToBe( newPosition ); + expect( await getDiff( newPosition ) ).toBeLessThanOrEqual( BUFFER ); // Type until the text wraps. while ( await page.evaluate( () => @@ -61,27 +54,25 @@ describe( 'TypeWriter', () => { await page.keyboard.type( 'a' ); } - await expectScrollPositionToBe( newPosition ); + expect( await getDiff( newPosition ) ).toBeLessThanOrEqual( BUFFER ); // Pressing backspace will reposition the caret to the previous line. // Scroll position should be adjusted again. await page.keyboard.press( 'Backspace' ); - await expectScrollPositionToBe( newPosition ); + expect( await getDiff( newPosition ) ).toBeLessThanOrEqual( BUFFER ); // Should reset scroll position to maintain. await page.keyboard.press( 'ArrowUp' ); - const positionAfterArrowUp = await page.evaluate( () => - wp.dom.computeCaretRect().y - ); + const positionAfterArrowUp = await getCaretPosition(); expect( positionAfterArrowUp ).toBeLessThan( newPosition ); // Should be scrolled to new position. await page.keyboard.press( 'Enter' ); - await expectScrollPositionToBe( positionAfterArrowUp ); + expect( await getDiff( positionAfterArrowUp ) ).toBeLessThanOrEqual( BUFFER ); } ); it( 'should maintain caret position after scroll', async () => { @@ -96,14 +87,12 @@ describe( 'TypeWriter', () => { // eslint-disable-next-line no-restricted-syntax await page.waitFor( 100 ); - const initialPosition = await page.evaluate( () => - wp.dom.computeCaretRect().y - ); + const initialPosition = await getCaretPosition(); // Should maintain scroll position. await page.keyboard.press( 'Enter' ); - await expectScrollPositionToBe( initialPosition ); + expect( await getDiff( initialPosition ) ).toBeLessThanOrEqual( BUFFER ); } ); it( 'should maintain caret position after leaving last editable', async () => { @@ -114,13 +103,82 @@ describe( 'TypeWriter', () => { // Move to first block. await page.keyboard.press( 'ArrowUp' ); - const initialPosition = await page.evaluate( () => - wp.dom.computeCaretRect().y - ); + const initialPosition = await getCaretPosition(); // Should maintain scroll position. await page.keyboard.press( 'Enter' ); - await expectScrollPositionToBe( initialPosition ); + expect( await getDiff( initialPosition ) ).toBeLessThanOrEqual( BUFFER ); + } ); + + it( 'should scroll caret into view from the top', async () => { + // Create first block. + await page.keyboard.press( 'Enter' ); + + let count = 0; + + // Create blocks until the the typewriter effect kicks in. + while ( await page.evaluate( () => + wp.dom.getScrollContainer( document.activeElement ).scrollTop === 0 + ) ) { + await page.keyboard.press( 'Enter' ); + count++; + } + + // Scroll the active element to the very bottom of the scroll container, + // then scroll 20px down, so the caret is partially hidden. + await page.evaluate( () => { + document.activeElement.scrollIntoView( false ); + wp.dom.getScrollContainer( document.activeElement ).scrollTop -= 20; + } ); + + const bottomPostition = await getCaretPosition(); + + // Should scroll the caret back into view (preserve browser behaviour). + await page.keyboard.type( 'a' ); + + // Debounce time for the scroll event listener. + // eslint-disable-next-line no-restricted-syntax + await page.waitFor( 100 ); + + const newBottomPosition = await getCaretPosition(); + + expect( newBottomPosition ).toBeLessThan( bottomPostition ); + + // Should maintain new caret position. + await page.keyboard.press( 'Enter' ); + + expect( await getDiff( newBottomPosition ) ).toBeLessThanOrEqual( BUFFER ); + + await page.keyboard.press( 'Backspace' ); + + while ( count-- ) { + await page.keyboard.press( 'ArrowUp' ); + } + + // Scroll the active element to the very top of the scroll container, + // then scroll 10px down, so the caret is partially hidden. + await page.evaluate( () => { + document.activeElement.scrollIntoView(); + wp.dom.getScrollContainer( document.activeElement ).scrollTop += 20; + } ); + + const topPostition = await getCaretPosition(); + + // Should scroll the caret back into view (preserve browser behaviour). + await page.keyboard.type( 'a' ); + + // Debounce time for the scroll event listener. + // eslint-disable-next-line no-restricted-syntax + await page.waitFor( 100 ); + + const newTopPosition = await getCaretPosition(); + + expect( newTopPosition ).toBeGreaterThan( topPostition ); + + // Should maintain new caret position. + await page.keyboard.press( 'Enter' ); + + expect( await getDiff( newTopPosition ) ).toBeLessThanOrEqual( BUFFER ); } ); } ); From 3ba1ceea44d248b8e54cdb0f265d00905396d7b5 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 19 Aug 2019 23:49:58 +0200 Subject: [PATCH 07/11] Clean up --- .../src/components/typewriter/index.js | 131 +++++++++--------- 1 file changed, 66 insertions(+), 65 deletions(-) diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js index 8c70cacf3f5165..7c870d522281f4 100644 --- a/packages/block-editor/src/components/typewriter/index.js +++ b/packages/block-editor/src/components/typewriter/index.js @@ -76,6 +76,14 @@ class Typewriter extends Component { ); } + isLastEditableNode() { + const editableNodes = this.ref.current.querySelectorAll( + '[contenteditable="true"]' + ); + const lastEditableNode = editableNodes[ editableNodes.length - 1 ]; + return lastEditableNode === document.activeElement; + } + /** * Maintains the scroll position after a selection change caused by a * keyboard event. @@ -83,10 +91,20 @@ class Typewriter extends Component { * @param {SyntheticEvent} event Synthetic keyboard event. */ maintainCaretPosition( { keyCode } ) { + if ( ! this.isSelectionEligibleForScroll() ) { + return; + } + + const currentCaretRect = computeCaretRect(); + + if ( ! currentCaretRect ) { + return; + } + // If for some reason there is no position set to be scrolled to, let // this be the position to be scrolled to in the future. if ( ! this.caretRect ) { - this.computeCaretRect(); + this.caretRect = currentCaretRect; return; } @@ -94,17 +112,8 @@ class Typewriter extends Component { // a pleasant experience, it may not be the case for everyone, so, for // now, let's disable it. if ( arrowKeyCodes.has( keyCode ) ) { - this.computeCaretRect(); - return; - } - - if ( ! this.isSelectionEligibleForScroll() ) { - return; - } - - const currentCaretRect = computeCaretRect(); - - if ( ! currentCaretRect ) { + // Reset the caret position to maintain. + this.caretRect = currentCaretRect; return; } @@ -121,63 +130,55 @@ class Typewriter extends Component { return; } - const editableNodes = this.ref.current.querySelectorAll( '[contenteditable="true"]' ); - const lastEditableNode = editableNodes[ editableNodes.length - 1 ]; - const isLastEditableNode = lastEditableNode === document.activeElement; - const scrollContainerRect = scrollContainer.getBoundingClientRect(); - const relativeScrollPosition = ( - ( this.caretRect.y - scrollContainerRect.y ) / - ( window.innerHeight - scrollContainerRect.y ) - ); - - // The scroll container may be different depending on the viewport - // width. - // Nested condition: if the scroll position is at the start and the - // active editable element is the last one, do not scroll the page. + const windowScroll = scrollContainer === document.body; + const scrollY = windowScroll ? + window.scrollY : + scrollContainer.scrollTop; + const scrollContainerY = windowScroll ? + 0 : + scrollContainer.getBoundingClientRect().y; + const relativeScrollPosition = windowScroll ? + this.caretRect.y / window.innerHeight : + ( this.caretRect.y - scrollContainerY ) / + ( window.innerHeight - scrollContainerY ); + + // If the scroll position is at the start, the active editable element + // is the last one, and the caret is positioned within the initial + // trigger percentage of the page, do not scroll the page. // The typewriter effect should not kick in until an empty page has been - // filled or the user scrolls intentionally down. - if ( scrollContainer === document.body ) { - if ( - isLastEditableNode && - window.scrollY === 0 && - relativeScrollPosition < initialTriggerPercentage - ) { - this.caretRect = currentCaretRect; - return; - } - - // Abort if the target scroll position would scroll the caret out of - // view. - if ( - this.caretRect.y + this.caretRect.height > window.innerHeight || - this.caretRect.y < 0 - ) { - this.caretRect = currentCaretRect; - return; - } + // filled with the initial trigger percentage or the user scrolls + // intentionally down. + if ( + scrollY === 0 && + relativeScrollPosition < initialTriggerPercentage && + this.isLastEditableNode() + ) { + // Reset the caret position to maintain. + this.caretRect = currentCaretRect; + return; + } + const scrollContainerHeight = windowScroll ? + window.innerHeight : + scrollContainer.clientHeight; + + // Abort if the target scroll position would scroll the caret out of + // view. + if ( + // The caret is under the lower fold. + this.caretRect.y + this.caretRect.height > + scrollContainerY + scrollContainerHeight || + // The caret is above the upper fold. + this.caretRect.y < scrollContainerY + ) { + // Reset the caret position to maintain. + this.caretRect = currentCaretRect; + return; + } + + if ( windowScroll ) { window.scrollBy( 0, diff ); } else { - if ( - isLastEditableNode && - scrollContainer.scrollTop === 0 && - relativeScrollPosition < initialTriggerPercentage - ) { - this.caretRect = currentCaretRect; - return; - } - - // Abort if the target scroll position would scroll the caret out of - // view. - if ( - this.caretRect.y + this.caretRect.height > - scrollContainerRect.y + scrollContainer.clientHeight || - this.caretRect.y < scrollContainerRect.y - ) { - this.caretRect = currentCaretRect; - return; - } - scrollContainer.scrollTop += diff; } } From 748382469faf81884b983743925ccbb388b14baf Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 20 Aug 2019 00:00:38 +0200 Subject: [PATCH 08/11] Adjust doc --- packages/block-editor/README.md | 3 ++- packages/block-editor/src/components/typewriter/index.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/README.md b/packages/block-editor/README.md index c988010de4e624..6c2fc3bbac7770 100644 --- a/packages/block-editor/README.md +++ b/packages/block-editor/README.md @@ -423,7 +423,8 @@ _Returns_ # **Typewriter** Ensures that the text selection keeps the same vertical distance from the -viewport during keyboard events within this component. +viewport during keyboard events within this component. The vertical distance +can vary. It is the last clicked or scrolled to position. # **URLInput** diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js index 7c870d522281f4..cde7c4a6300a0c 100644 --- a/packages/block-editor/src/components/typewriter/index.js +++ b/packages/block-editor/src/components/typewriter/index.js @@ -228,7 +228,8 @@ class Typewriter extends Component { /** * Ensures that the text selection keeps the same vertical distance from the - * viewport during keyboard events within this component. + * viewport during keyboard events within this component. The vertical distance + * can vary. It is the last clicked or scrolled to position. */ export default withSelect( ( select ) => { const { getSelectedBlockClientId } = select( 'core/block-editor' ); From 51fef27e6726acc9b5c28d2a4730864073e2054e Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 20 Aug 2019 10:29:41 +0200 Subject: [PATCH 09/11] Return children for IE --- packages/block-editor/src/components/typewriter/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js index cde7c4a6300a0c..625789c449e281 100644 --- a/packages/block-editor/src/components/typewriter/index.js +++ b/packages/block-editor/src/components/typewriter/index.js @@ -205,7 +205,7 @@ class Typewriter extends Component { // There are some issues with Internet Explorer, which are probably not // worth spending time on. Let's disable it. if ( isIE ) { - return null; + return this.props.children; } // Disable reason: Wrapper itself is non-interactive, but must capture From 5031b5068b26ee18d3d5c481184b14fb069a6f70 Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 20 Aug 2019 11:29:16 +0200 Subject: [PATCH 10/11] Hide bottom space when there are metaboxes --- packages/edit-post/src/components/layout/index.js | 1 + packages/edit-post/src/components/visual-editor/style.scss | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/packages/edit-post/src/components/layout/index.js b/packages/edit-post/src/components/layout/index.js index ee2b55bbed5d24..4ae2c2a9a71e9b 100644 --- a/packages/edit-post/src/components/layout/index.js +++ b/packages/edit-post/src/components/layout/index.js @@ -62,6 +62,7 @@ function Layout( { const className = classnames( 'edit-post-layout', { 'is-sidebar-opened': sidebarIsOpened, 'has-fixed-toolbar': hasFixedToolbar, + 'has-metaboxes': hasActiveMetaboxes, } ); const publishLandmarkProps = { diff --git a/packages/edit-post/src/components/visual-editor/style.scss b/packages/edit-post/src/components/visual-editor/style.scss index fb3bf165b4a326..6be41449eae792 100644 --- a/packages/edit-post/src/components/visual-editor/style.scss +++ b/packages/edit-post/src/components/visual-editor/style.scss @@ -13,6 +13,11 @@ width: 100%; } +// Hide the extra space when there are metaboxes. +.has-metaboxes .edit-post-visual-editor .block-editor-writing-flow__click-redirect { + height: 0; +} + // The base width of blocks .edit-post-visual-editor .block-editor-block-list__block { margin-left: auto; From 3d7e9e15ca485fd06e955602cba8ab2cc53b1b96 Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 20 Aug 2019 13:30:27 +0200 Subject: [PATCH 11/11] Debounce scroll with RAF --- .../src/components/typewriter/index.js | 49 +++++++++++++------ packages/e2e-tests/specs/typewriter.test.js | 16 ------ 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/packages/block-editor/src/components/typewriter/index.js b/packages/block-editor/src/components/typewriter/index.js index 625789c449e281..cb17e38e549200 100644 --- a/packages/block-editor/src/components/typewriter/index.js +++ b/packages/block-editor/src/components/typewriter/index.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import { debounce } from 'lodash'; - /** * WordPress dependencies */ @@ -25,23 +20,29 @@ class Typewriter extends Component { this.computeCaretRectOnSelectionChange = this.computeCaretRectOnSelectionChange.bind( this ); this.maintainCaretPosition = this.maintainCaretPosition.bind( this ); this.computeCaretRect = this.computeCaretRect.bind( this ); - this.debouncedComputeCaretRect = debounce( this.computeCaretRect, 100 ); + this.onScrollResize = this.onScrollResize.bind( this ); this.isSelectionEligibleForScroll = this.isSelectionEligibleForScroll.bind( this ); } componentDidMount() { // When the user scrolls or resizes, the scroll position should be // reset. - window.addEventListener( 'scroll', this.debouncedComputeCaretRect, true ); - window.addEventListener( 'resize', this.debouncedComputeCaretRect, true ); + window.addEventListener( 'scroll', this.onScrollResize, true ); + window.addEventListener( 'resize', this.onScrollResize, true ); } componentWillUnmount() { - window.removeEventListener( 'scroll', this.debouncedComputeCaretRect, true ); - window.removeEventListener( 'resize', this.debouncedComputeCaretRect, true ); + window.removeEventListener( 'scroll', this.onScrollResize, true ); + window.removeEventListener( 'resize', this.onScrollResize, true ); document.removeEventListener( 'selectionchange', this.computeCaretRectOnSelectionChange ); - window.cancelAnimationFrame( this.rafId ); - this.debouncedComputeCaretRect.cancel(); + + if ( this.onScrollResize.rafId ) { + window.cancelAnimationFrame( this.onScrollResize.rafId ); + } + + if ( this.onKeyDown.rafId ) { + window.cancelAnimationFrame( this.onKeyDown.rafId ); + } } /** @@ -62,6 +63,17 @@ class Typewriter extends Component { this.computeCaretRect(); } + onScrollResize() { + if ( this.onScrollResize.rafId ) { + return; + } + + this.onScrollResize.rafId = window.requestAnimationFrame( () => { + this.computeCaretRect(); + delete this.onScrollResize.rafId; + } ); + } + /** * Checks if the current situation is elegible for scroll: * - There should be one and only one block selected. @@ -193,12 +205,17 @@ class Typewriter extends Component { onKeyDown( event ) { event.persist(); + // Ensure the any remaining request is cancelled. - window.cancelAnimationFrame( this.rafId ); + if ( this.onKeyDown.rafId ) { + window.cancelAnimationFrame( this.onKeyDown.rafId ); + } + // Use an animation frame for a smooth result. - this.rafId = window.requestAnimationFrame( () => - this.maintainCaretPosition( event ) - ); + this.onKeyDown.rafId = window.requestAnimationFrame( () => { + this.maintainCaretPosition( event ); + delete this.onKeyDown.rafId; + } ); } render() { diff --git a/packages/e2e-tests/specs/typewriter.test.js b/packages/e2e-tests/specs/typewriter.test.js index dd386ee7f9e8f7..a41d9b0ba4b71b 100644 --- a/packages/e2e-tests/specs/typewriter.test.js +++ b/packages/e2e-tests/specs/typewriter.test.js @@ -35,10 +35,6 @@ describe( 'TypeWriter', () => { await page.keyboard.press( 'Enter' ); } - // Debounce time for the scroll event listener. - // eslint-disable-next-line no-restricted-syntax - await page.waitFor( 100 ); - const newPosition = await getCaretPosition(); // Now the scroll position should be maintained. @@ -83,10 +79,6 @@ describe( 'TypeWriter', () => { wp.dom.getScrollContainer( document.activeElement ).scrollTop = 1 ); - // Debounce time for the scroll event listener. - // eslint-disable-next-line no-restricted-syntax - await page.waitFor( 100 ); - const initialPosition = await getCaretPosition(); // Should maintain scroll position. @@ -137,10 +129,6 @@ describe( 'TypeWriter', () => { // Should scroll the caret back into view (preserve browser behaviour). await page.keyboard.type( 'a' ); - // Debounce time for the scroll event listener. - // eslint-disable-next-line no-restricted-syntax - await page.waitFor( 100 ); - const newBottomPosition = await getCaretPosition(); expect( newBottomPosition ).toBeLessThan( bottomPostition ); @@ -168,10 +156,6 @@ describe( 'TypeWriter', () => { // Should scroll the caret back into view (preserve browser behaviour). await page.keyboard.type( 'a' ); - // Debounce time for the scroll event listener. - // eslint-disable-next-line no-restricted-syntax - await page.waitFor( 100 ); - const newTopPosition = await getCaretPosition(); expect( newTopPosition ).toBeGreaterThan( topPostition );