From c49b051f3340cccdc59d140b44b450b87239cb01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= <wp@iseulde.com>
Date: Tue, 28 Jan 2020 11:58:32 +0200
Subject: [PATCH] Block editor: Alt+F10 shouldn't scroll to top (#19896)

* Add e2e test

* Leave fixed position until position can be set
---
 .../components/block-list/insertion-point.js  |  1 +
 .../src/components/block-list/style.scss      |  2 --
 packages/components/src/popover/index.js      | 10 +++++--
 .../editor/various/navigable-toolbar.test.js  | 26 ++++++++++++++++++-
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/packages/block-editor/src/components/block-list/insertion-point.js b/packages/block-editor/src/components/block-list/insertion-point.js
index 336887375b527c..7ca4d53ca065ce 100644
--- a/packages/block-editor/src/components/block-list/insertion-point.js
+++ b/packages/block-editor/src/components/block-list/insertion-point.js
@@ -124,6 +124,7 @@ export default function InsertionPoint( {
 			focusOnMount={ false }
 			className="block-editor-block-list__insertion-point-popover"
 			__unstableSlotName="block-toolbar"
+			__unstableFixedPosition={ false }
 		>
 			<div className="block-editor-block-list__insertion-point" style={ { width: inserterElement.offsetWidth } }>
 				<Indicator clientId={ inserterClientId } />
diff --git a/packages/block-editor/src/components/block-list/style.scss b/packages/block-editor/src/components/block-list/style.scss
index 173225ceab1ecd..4fe3262ed024a6 100644
--- a/packages/block-editor/src/components/block-list/style.scss
+++ b/packages/block-editor/src/components/block-list/style.scss
@@ -539,7 +539,6 @@
 
 .block-editor-block-list__insertion-point-popover {
 	z-index: z-index(".block-editor-block-list__insertion-point-popover");
-	position: absolute;
 
 	.components-popover__content {
 		background: none;
@@ -551,7 +550,6 @@
 
 .components-popover.block-editor-block-list__block-popover {
 	z-index: z-index(".block-editor-block-list__block-popover");
-	position: absolute;
 
 	.components-popover__content {
 		margin: 0 !important;
diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js
index 74aeef76d52068..88944369cc3ea0 100644
--- a/packages/components/src/popover/index.js
+++ b/packages/components/src/popover/index.js
@@ -246,6 +246,7 @@ const Popover = ( {
 	__unstableSlotName = SLOT_NAME,
 	__unstableAllowVerticalSubpixelPosition,
 	__unstableAllowHorizontalSubpixelPosition,
+	__unstableFixedPosition = true,
 	/* eslint-enable no-unused-vars */
 	...contentProps
 } ) => {
@@ -268,6 +269,7 @@ const Popover = ( {
 			setStyle( containerRef.current, 'left' );
 			setStyle( contentRef.current, 'maxHeight' );
 			setStyle( contentRef.current, 'maxWidth' );
+			setStyle( containerRef.current, 'position' );
 			return;
 		}
 
@@ -292,7 +294,6 @@ const Popover = ( {
 				contentRect.current = contentRef.current.getBoundingClientRect();
 			}
 
-			const { offsetParent, ownerDocument } = containerRef.current;
 			let relativeOffsetTop = 0;
 
 			// If there is a positioned ancestor element that is not the body,
@@ -300,7 +301,10 @@ const Popover = ( {
 			// the popover is fixed, the offset parent is null or the body
 			// element, in which case the position is relative to the viewport.
 			// See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetParent
-			if ( offsetParent && offsetParent !== ownerDocument.body ) {
+			if ( ! __unstableFixedPosition ) {
+				setStyle( containerRef.current, 'position', 'absolute' );
+
+				const { offsetParent } = containerRef.current;
 				const offsetParentRect = offsetParent.getBoundingClientRect();
 
 				relativeOffsetTop = offsetParentRect.top;
@@ -310,6 +314,8 @@ const Popover = ( {
 					anchor.width,
 					anchor.height
 				);
+			} else {
+				setStyle( containerRef.current, 'position' );
 			}
 
 			const {
diff --git a/packages/e2e-tests/specs/editor/various/navigable-toolbar.test.js b/packages/e2e-tests/specs/editor/various/navigable-toolbar.test.js
index 1b8efda52e1bb9..62403ee6da796f 100644
--- a/packages/e2e-tests/specs/editor/various/navigable-toolbar.test.js
+++ b/packages/e2e-tests/specs/editor/various/navigable-toolbar.test.js
@@ -40,6 +40,30 @@ describe.each( [ [ 'unified', true ], [ 'contextual', false ] ] )(
 			await pressKeyWithModifier( 'alt', 'F10' );
 			expect( await isInBlockToolbar() ).toBe( true );
 		} );
+
+		if ( ! isUnifiedToolbar ) {
+			it( 'should not scroll page', async () => {
+				while ( await page.evaluate( () =>
+					wp.dom.getScrollContainer( document.activeElement ).scrollTop === 0
+				) ) {
+					await page.keyboard.press( 'Enter' );
+				}
+
+				await page.keyboard.type( 'a' );
+
+				const scrollTopBefore = await page.evaluate( () =>
+					wp.dom.getScrollContainer( document.activeElement ).scrollTop
+				);
+
+				await pressKeyWithModifier( 'alt', 'F10' );
+				expect( await isInBlockToolbar() ).toBe( true );
+
+				const scrollTopAfter = await page.evaluate( () =>
+					wp.dom.getScrollContainer( document.activeElement ).scrollTop
+				);
+
+				expect( scrollTopBefore ).toBe( scrollTopAfter );
+			} );
+		}
 	}
 );
-