From ba9c763954276e18b6f577f721d0330d2eeb494b Mon Sep 17 00:00:00 2001
From: Dan <dan.abramov@gmail.com>
Date: Sat, 13 Apr 2019 01:58:05 +0100
Subject: [PATCH 1/3] Consistently scroll component name into view

---
 src/devtools/views/Components/Element.css |  5 +++++
 src/devtools/views/Components/Element.js  | 23 +++++++++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/src/devtools/views/Components/Element.css b/src/devtools/views/Components/Element.css
index 2a923c15d9..425b901bd8 100644
--- a/src/devtools/views/Components/Element.css
+++ b/src/devtools/views/Components/Element.css
@@ -17,6 +17,11 @@
   background-color: var(--color-inactive-background);
 }
 
+.ScrollAnchor {
+  height: 100%;
+  width: 0;
+}
+
 .SelectedElement {
   background-color: var(--color-selected-background);
   color: var(--color-selected-foreground);
diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js
index 16fb243ff2..bf81291f4a 100644
--- a/src/devtools/views/Components/Element.js
+++ b/src/devtools/views/Components/Element.js
@@ -56,7 +56,8 @@ export default function ElementView({ data, index, style }: Props) {
     }
   }, [id, selectOwner]);
 
-  const ref = useRef<HTMLSpanElement | null>(null);
+  const scrollAnchorStartRef = useRef<HTMLSpanElement | null>(null);
+  const scrollAnchorEndRef = useRef<HTMLSpanElement | null>(null);
 
   // The tree above has its own autoscrolling, but it only works for rows.
   // However, even when the row gets into the viewport, the component name
@@ -74,8 +75,20 @@ export default function ElementView({ data, index, style }: Props) {
       }
       lastScrolledIDRef.current = id;
 
-      if (ref.current !== null) {
-        ref.current.scrollIntoView({
+      // We want to bring the whole <Component> name into view,
+      // including the expansion toggle and the "=== $r" hint.
+      // However, even calling scrollIntoView() on their parent node
+      // wouldn't guarantee that it will be *fully* brought into view.
+      // As a workaround, we'll have two anchor spans, and scroll each into view.
+      if (scrollAnchorStartRef.current !== null) {
+        scrollAnchorStartRef.current.scrollIntoView({
+          behavior: 'auto',
+          block: 'nearest',
+          inline: 'nearest',
+        });
+      }
+      if (scrollAnchorEndRef.current !== null) {
+        scrollAnchorEndRef.current.scrollIntoView({
           behavior: 'auto',
           block: 'nearest',
           inline: 'nearest',
@@ -149,10 +162,11 @@ export default function ElementView({ data, index, style }: Props) {
         marginBottom: `-${style.height}px`,
       }}
     >
+      <span className={styles.ScrollAnchor} ref={scrollAnchorStartRef} />
       {ownerStack.length === 0 ? (
         <ExpandCollapseToggle element={element} store={store} />
       ) : null}
-      <span className={styles.Component} ref={ref}>
+      <span className={styles.Component}>
         <DisplayName displayName={displayName} id={((id: any): number)} />
         {key && (
           <Fragment>
@@ -162,6 +176,7 @@ export default function ElementView({ data, index, style }: Props) {
         )}
       </span>
       {showDollarR && <span className={styles.DollarR}>&nbsp;== $r</span>}
+      <span className={styles.ScrollAnchor} ref={scrollAnchorEndRef} />
     </div>
   );
 }

From 9162b7165ad446cf12cea88eecf94f2787de6e75 Mon Sep 17 00:00:00 2001
From: Dan <dan.abramov@gmail.com>
Date: Sat, 13 Apr 2019 16:49:10 +0100
Subject: [PATCH 2/3] Prefer to keep the start anchor visible

---
 src/devtools/views/Components/Element.js | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js
index bf81291f4a..48efeba641 100644
--- a/src/devtools/views/Components/Element.js
+++ b/src/devtools/views/Components/Element.js
@@ -80,15 +80,17 @@ export default function ElementView({ data, index, style }: Props) {
       // However, even calling scrollIntoView() on their parent node
       // wouldn't guarantee that it will be *fully* brought into view.
       // As a workaround, we'll have two anchor spans, and scroll each into view.
-      if (scrollAnchorStartRef.current !== null) {
-        scrollAnchorStartRef.current.scrollIntoView({
+      if (scrollAnchorEndRef.current !== null) {
+        scrollAnchorEndRef.current.scrollIntoView({
           behavior: 'auto',
           block: 'nearest',
           inline: 'nearest',
         });
       }
-      if (scrollAnchorEndRef.current !== null) {
-        scrollAnchorEndRef.current.scrollIntoView({
+      if (scrollAnchorStartRef.current !== null) {
+        // We scroll the start anchor last because it's
+        // more important for it to be in the view.
+        scrollAnchorStartRef.current.scrollIntoView({
           behavior: 'auto',
           block: 'nearest',
           inline: 'nearest',

From 518f3d75f8eba06a44dc2e208c49f569b2d2cb3d Mon Sep 17 00:00:00 2001
From: Brian Vaughn <brian.david.vaughn@gmail.com>
Date: Sat, 13 Apr 2019 15:38:56 -0700
Subject: [PATCH 3/3] Tweaked an inline comment.

---
 src/devtools/views/Components/Element.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js
index 48efeba641..da51484fbe 100644
--- a/src/devtools/views/Components/Element.js
+++ b/src/devtools/views/Components/Element.js
@@ -77,7 +77,7 @@ export default function ElementView({ data, index, style }: Props) {
 
       // We want to bring the whole <Component> name into view,
       // including the expansion toggle and the "=== $r" hint.
-      // However, even calling scrollIntoView() on their parent node
+      // However, even calling scrollIntoView() on a wrapper parent node (e.g. <span>)
       // wouldn't guarantee that it will be *fully* brought into view.
       // As a workaround, we'll have two anchor spans, and scroll each into view.
       if (scrollAnchorEndRef.current !== null) {