From 6d54e045f68fef15b7a56073e5d490d47fd34a43 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= <ella@vandurpe.com>
Date: Fri, 7 May 2021 16:24:50 +0300
Subject: [PATCH] Forward refs

---
 .../components/block-tools/block-popover.js   | 111 +++++++++---------
 .../components/block-tools/insertion-point.js |   9 +-
 .../block-tools/use-popover-scroll.js         |   7 +-
 packages/components/src/popover/index.js      |  88 ++++++++------
 .../src/slot-fill/bubbles-virtually/slot.js   |  24 ++--
 packages/components/src/slot-fill/index.js    |  12 +-
 6 files changed, 141 insertions(+), 110 deletions(-)

diff --git a/packages/block-editor/src/components/block-tools/block-popover.js b/packages/block-editor/src/components/block-tools/block-popover.js
index 6c637e0331b2a2..dc803f7670aba6 100644
--- a/packages/block-editor/src/components/block-tools/block-popover.js
+++ b/packages/block-editor/src/components/block-tools/block-popover.js
@@ -178,6 +178,7 @@ function BlockPopover( {
 
 	return (
 		<Popover
+			ref={ popoverScrollRef }
 			noArrow
 			animate={ false }
 			position={ popoverPosition }
@@ -193,66 +194,64 @@ function BlockPopover( {
 			__unstableObserveElement={ node }
 			shouldAnchorIncludePadding
 		>
-			<div ref={ popoverScrollRef }>
-				{ ( shouldShowContextualToolbar || isToolbarForced ) && (
-					<div
-						onFocus={ onFocus }
-						onBlur={ onBlur }
-						// While ideally it would be enough to capture the
-						// bubbling focus event from the Inserter, due to the
-						// characteristics of click focusing of `button`s in
-						// Firefox and Safari, it is not reliable.
-						//
-						// See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus
-						tabIndex={ -1 }
-						className={ classnames(
-							'block-editor-block-list__block-popover-inserter',
-							{
-								'is-visible': isInserterShown,
-							}
-						) }
-					>
-						<Inserter
-							clientId={ clientId }
-							rootClientId={ rootClientId }
-							__experimentalIsQuick
-						/>
-					</div>
-				) }
-				{ ( shouldShowContextualToolbar || isToolbarForced ) && (
-					<BlockContextualToolbar
-						// If the toolbar is being shown because of being forced
-						// it should focus the toolbar right after the mount.
-						focusOnMount={ isToolbarForced }
-						__experimentalInitialIndex={
-							initialToolbarItemIndexRef.current
+			{ ( shouldShowContextualToolbar || isToolbarForced ) && (
+				<div
+					onFocus={ onFocus }
+					onBlur={ onBlur }
+					// While ideally it would be enough to capture the
+					// bubbling focus event from the Inserter, due to the
+					// characteristics of click focusing of `button`s in
+					// Firefox and Safari, it is not reliable.
+					//
+					// See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus
+					tabIndex={ -1 }
+					className={ classnames(
+						'block-editor-block-list__block-popover-inserter',
+						{
+							'is-visible': isInserterShown,
 						}
-						__experimentalOnIndexChange={ ( index ) => {
-							initialToolbarItemIndexRef.current = index;
-						} }
-						// Resets the index whenever the active block changes so
-						// this is not persisted. See https://github.com/WordPress/gutenberg/pull/25760#issuecomment-717906169
-						key={ clientId }
-					/>
-				) }
-				{ shouldShowBreadcrumb && (
-					<BlockSelectionButton
+					) }
+				>
+					<Inserter
 						clientId={ clientId }
 						rootClientId={ rootClientId }
-						blockElement={ node }
+						__experimentalIsQuick
+					/>
+				</div>
+			) }
+			{ ( shouldShowContextualToolbar || isToolbarForced ) && (
+				<BlockContextualToolbar
+					// If the toolbar is being shown because of being forced
+					// it should focus the toolbar right after the mount.
+					focusOnMount={ isToolbarForced }
+					__experimentalInitialIndex={
+						initialToolbarItemIndexRef.current
+					}
+					__experimentalOnIndexChange={ ( index ) => {
+						initialToolbarItemIndexRef.current = index;
+					} }
+					// Resets the index whenever the active block changes so
+					// this is not persisted. See https://github.com/WordPress/gutenberg/pull/25760#issuecomment-717906169
+					key={ clientId }
+				/>
+			) }
+			{ shouldShowBreadcrumb && (
+				<BlockSelectionButton
+					clientId={ clientId }
+					rootClientId={ rootClientId }
+					blockElement={ node }
+				/>
+			) }
+			{ showEmptyBlockSideInserter && (
+				<div className="block-editor-block-list__empty-block-inserter">
+					<Inserter
+						position="bottom right"
+						rootClientId={ rootClientId }
+						clientId={ clientId }
+						__experimentalIsQuick
 					/>
-				) }
-				{ showEmptyBlockSideInserter && (
-					<div className="block-editor-block-list__empty-block-inserter">
-						<Inserter
-							position="bottom right"
-							rootClientId={ rootClientId }
-							clientId={ clientId }
-							__experimentalIsQuick
-						/>
-					</div>
-				) }
-			</div>
+				</div>
+			) }
 		</Popover>
 	);
 }
diff --git a/packages/block-editor/src/components/block-tools/insertion-point.js b/packages/block-editor/src/components/block-tools/insertion-point.js
index 380ef7c75cd484..134430c301a61e 100644
--- a/packages/block-editor/src/components/block-tools/insertion-point.js
+++ b/packages/block-editor/src/components/block-tools/insertion-point.js
@@ -16,7 +16,6 @@ import {
 } from '@wordpress/element';
 import { Popover } from '@wordpress/components';
 import { isRTL } from '@wordpress/i18n';
-import { useMergeRefs } from '@wordpress/compose';
 
 /**
  * Internal dependencies
@@ -166,10 +165,7 @@ function InsertionPointPopover( {
 		};
 	}, [ previousElement, nextElement ] );
 
-	const mergedRefs = useMergeRefs( [
-		ref,
-		usePopoverScroll( __unstableContentRef ),
-	] );
+	const popoverScrollRef = usePopoverScroll( __unstableContentRef );
 
 	if ( ! previousElement ) {
 		return null;
@@ -215,6 +211,7 @@ function InsertionPointPopover( {
 	// See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus
 	return (
 		<Popover
+			ref={ popoverScrollRef }
 			noArrow
 			animate={ false }
 			getAnchorRect={ getAnchorRect }
@@ -225,7 +222,7 @@ function InsertionPointPopover( {
 			__unstableSlotName={ __unstablePopoverSlot || null }
 		>
 			<div
-				ref={ mergedRefs }
+				ref={ ref }
 				tabIndex={ -1 }
 				onClick={ onClick }
 				onFocus={ onFocus }
diff --git a/packages/block-editor/src/components/block-tools/use-popover-scroll.js b/packages/block-editor/src/components/block-tools/use-popover-scroll.js
index c6934b05952ffd..4730ea629b0956 100644
--- a/packages/block-editor/src/components/block-tools/use-popover-scroll.js
+++ b/packages/block-editor/src/components/block-tools/use-popover-scroll.js
@@ -13,7 +13,12 @@ import { useRefEffect } from '@wordpress/compose';
 export function usePopoverScroll( scrollableRef ) {
 	return useRefEffect(
 		( node ) => {
-			function onWheel( { deltaX, deltaY } ) {
+			if ( ! scrollableRef ) {
+				return;
+			}
+
+			function onWheel( event ) {
+				const { deltaX, deltaY } = event;
 				scrollableRef.current.scrollBy( deltaX, deltaY );
 			}
 			node.addEventListener( 'wheel', onWheel );
diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js
index 5833a836056363..b0e8d4bccb7a37 100644
--- a/packages/components/src/popover/index.js
+++ b/packages/components/src/popover/index.js
@@ -6,7 +6,12 @@ import classnames from 'classnames';
 /**
  * WordPress dependencies
  */
-import { useRef, useState, useLayoutEffect } from '@wordpress/element';
+import {
+	useRef,
+	useState,
+	useLayoutEffect,
+	forwardRef,
+} from '@wordpress/element';
 import { getRectangleFromRange } from '@wordpress/dom';
 import { ESCAPE } from '@wordpress/keycodes';
 import deprecated from '@wordpress/deprecated';
@@ -223,36 +228,39 @@ function getAnchorDocument( anchor ) {
 	return anchor.ownerDocument;
 }
 
-const Popover = ( {
-	headerTitle,
-	onClose,
-	onKeyDown,
-	children,
-	className,
-	noArrow = true,
-	isAlternate,
-	// Disable reason: We generate the `...contentProps` rest as remainder
-	// of props which aren't explicitly handled by this component.
-	/* eslint-disable no-unused-vars */
-	position = 'bottom right',
-	range,
-	focusOnMount = 'firstElement',
-	anchorRef,
-	shouldAnchorIncludePadding,
-	anchorRect,
-	getAnchorRect,
-	expandOnMobile,
-	animate = true,
-	onClickOutside,
-	onFocusOutside,
-	__unstableStickyBoundaryElement,
-	__unstableSlotName = SLOT_NAME,
-	__unstableObserveElement,
-	__unstableBoundaryParent,
-	__unstableForcePosition,
-	/* eslint-enable no-unused-vars */
-	...contentProps
-} ) => {
+const Popover = (
+	{
+		headerTitle,
+		onClose,
+		onKeyDown,
+		children,
+		className,
+		noArrow = true,
+		isAlternate,
+		// Disable reason: We generate the `...contentProps` rest as remainder
+		// of props which aren't explicitly handled by this component.
+		/* eslint-disable no-unused-vars */
+		position = 'bottom right',
+		range,
+		focusOnMount = 'firstElement',
+		anchorRef,
+		shouldAnchorIncludePadding,
+		anchorRect,
+		getAnchorRect,
+		expandOnMobile,
+		animate = true,
+		onClickOutside,
+		onFocusOutside,
+		__unstableStickyBoundaryElement,
+		__unstableSlotName = SLOT_NAME,
+		__unstableObserveElement,
+		__unstableBoundaryParent,
+		__unstableForcePosition,
+		/* eslint-enable no-unused-vars */
+		...contentProps
+	},
+	ref
+) => {
 	const anchorRefFallback = useRef( null );
 	const contentRef = useRef( null );
 	const containerRef = useRef();
@@ -474,6 +482,7 @@ const Popover = ( {
 	const focusOnMountRef = useFocusOnMount( focusOnMount );
 	const focusOutsideProps = useFocusOutside( handleOnFocusOutside );
 	const mergedRefs = useMergeRefs( [
+		ref,
 		containerRef,
 		focusOnMount ? constrainedTabbingRef : null,
 		focusOnMount ? focusReturnRef : null,
@@ -616,10 +625,19 @@ const Popover = ( {
 	return <span ref={ anchorRefFallback }>{ content }</span>;
 };
 
-const PopoverContainer = Popover;
+const PopoverContainer = forwardRef( Popover );
+
+function PopoverSlot( { name = SLOT_NAME }, ref ) {
+	return (
+		<Slot
+			bubblesVirtually
+			name={ name }
+			className="popover-slot"
+			ref={ ref }
+		/>
+	);
+}
 
-PopoverContainer.Slot = ( { name = SLOT_NAME } ) => (
-	<Slot bubblesVirtually name={ name } className="popover-slot" />
-);
+PopoverContainer.Slot = forwardRef( PopoverSlot );
 
 export default PopoverContainer;
diff --git a/packages/components/src/slot-fill/bubbles-virtually/slot.js b/packages/components/src/slot-fill/bubbles-virtually/slot.js
index ace175e4b5a59e..5075677c3e173b 100644
--- a/packages/components/src/slot-fill/bubbles-virtually/slot.js
+++ b/packages/components/src/slot-fill/bubbles-virtually/slot.js
@@ -1,19 +1,23 @@
 /**
  * WordPress dependencies
  */
-import { useRef, useLayoutEffect, useContext } from '@wordpress/element';
+import {
+	useRef,
+	useLayoutEffect,
+	useContext,
+	forwardRef,
+} from '@wordpress/element';
+import { useMergeRefs } from '@wordpress/compose';
 
 /**
  * Internal dependencies
  */
 import SlotFillContext from './slot-fill-context';
 
-export default function Slot( {
-	name,
-	fillProps = {},
-	as: Component = 'div',
-	...props
-} ) {
+function Slot(
+	{ name, fillProps = {}, as: Component = 'div', ...props },
+	forwardedRef
+) {
 	const registry = useContext( SlotFillContext );
 	const ref = useRef();
 
@@ -34,5 +38,9 @@ export default function Slot( {
 		registry.updateSlot( name, fillProps );
 	} );
 
-	return <Component ref={ ref } { ...props } />;
+	return (
+		<Component ref={ useMergeRefs( [ forwardedRef, ref ] ) } { ...props } />
+	);
 }
+
+export default forwardRef( Slot );
diff --git a/packages/components/src/slot-fill/index.js b/packages/components/src/slot-fill/index.js
index 8c61fa2a444c3b..f91461a3385274 100644
--- a/packages/components/src/slot-fill/index.js
+++ b/packages/components/src/slot-fill/index.js
@@ -1,3 +1,8 @@
+/**
+ * WordPress dependencies
+ */
+import { forwardRef } from '@wordpress/element';
+
 /**
  * Internal dependencies
  */
@@ -20,13 +25,12 @@ export function Fill( props ) {
 		</>
 	);
 }
-
-export function Slot( { bubblesVirtually, ...props } ) {
+export const Slot = forwardRef( ( { bubblesVirtually, ...props }, ref ) => {
 	if ( bubblesVirtually ) {
-		return <BubblesVirtuallySlot { ...props } />;
+		return <BubblesVirtuallySlot { ...props } ref={ ref } />;
 	}
 	return <BaseSlot { ...props } />;
-}
+} );
 
 export function Provider( { children, ...props } ) {
 	return (