MBS-12936: Don't update Autocomplete2 scroll position on item mouseov…

…er (#2864)

In d4b15e9 I had intended to fix an issue
where highlighting an item lower down in the list (thereby updating the scroll
position), closing the menu (e.g. by hitting escape), and then re-opening the
menu would not bring the scroll position back to the top of the list.  While I
did fix that, there were some unintended effects when using a mouse: hovering
over any item would cause the menu to scroll towards that item.

The actual intent is that we should only update the scroll position when using
the arrow keys or when opening the menu. Obviously if a user can mouseover an
item, then it's already in view, and we don't need to mess with their manual
scrolling.  This commit fixes that and adds a Selenium test to make sure we
don't regress in this behavior again.

root/static/scripts/common/components/Autocomplete2.js

@@ -337,7 +337,7 @@ const Autocomplete2 = (React.memo(<+T: EntityItemT>(
   const inputTimeout = React.useRef<TimeoutID | null>(null);
   const containerRef = React.useRef<HTMLDivElement | null>(null);
   const prevIsOpen = React.useRef<boolean>(false);
-  const prevHighlightedIndex = React.useRef<number>(-1);
+  const shouldUpdateScrollPositionRef = React.useRef<boolean>(false);
 
   const highlightedItem = highlightedIndex >= 0
     ? (items[highlightedIndex] ?? null)
@@ -528,6 +528,7 @@ const Autocomplete2 = (React.memo(<+T: EntityItemT>(
         event.preventDefault();
 
         if (isOpen) {
+          shouldUpdateScrollPositionRef.current = true;
           dispatch(HIGHLIGHT_NEXT_ITEM);
         } else {
           showAvailableItemsOrBeginLookupOrSearch();
@@ -537,6 +538,7 @@ const Autocomplete2 = (React.memo(<+T: EntityItemT>(
       case 'ArrowUp':
         if (isOpen) {
           event.preventDefault();
+          shouldUpdateScrollPositionRef.current = true;
           dispatch(HIGHLIGHT_PREVIOUS_ITEM);
         }
         break;
@@ -676,22 +678,15 @@ const Autocomplete2 = (React.memo(<+T: EntityItemT>(
 
   React.useLayoutEffect(() => {
     const shouldUpdateScrollPosition = (
-      isOpen &&
-      (
-        !prevIsOpen.current ||
-        highlightedIndex !== prevHighlightedIndex.current
-      )
+      (isOpen && !prevIsOpen.current) ||
+      shouldUpdateScrollPositionRef.current
     );
     prevIsOpen.current = isOpen;
-    prevHighlightedIndex.current = highlightedIndex;
     if (shouldUpdateScrollPosition) {
       setScrollPosition(menuId);
+      shouldUpdateScrollPositionRef.current = false;
     }
-  }, [
-    isOpen,
-    highlightedIndex,
-    menuId,
-  ]);
+  });
 
   type AutocompleteItemComponent<T> =
     React$AbstractComponent<AutocompleteItemPropsT<T>, void>;

t/selenium.mjs

@@ -314,6 +314,7 @@ const KEY_CODES = {
   '${KEY_HOME}': Key.HOME,
   '${KEY_SHIFT}': Key.SHIFT,
   '${KEY_TAB}': Key.TAB,
+  '${KEY_UP}': Key.ARROW_UP,
   '${MBS_ROOT}': DBDefs.MB_SERVER_ROOT.replace(/\/$/, ''),
 };
 

t/selenium/Autocomplete2.json5

@@ -18,5 +18,108 @@
       value: '0',
     },
     // End of test for MBS-12631.
+    // MBS-12936: The menu scroll position should only be updated when
+    // navigating items using the arrow keys, or when initially opening the
+    // menu (to reset it to the top). Hovering over items with the mouse
+    // should explicitly not affect the scroll position.
+    // The menu should now be open; first check that the scroll position is
+    // initially zero:
+    {
+      command: 'assertEval',
+      target: 'document.querySelector("#attribute-type-test-menu").scrollTop',
+      value: '0',
+    },
+    // Simulate a mouseover event on the last item.
+    {
+      command: 'runScript',
+      target: '\
+        document.querySelector("#attribute-type-test-menu li:last-child")\
+          .dispatchEvent(new Event("mouseover", {bubbles: true}))\
+      ',
+      value: '',
+    },
+    // Make sure the last item is highlighted.
+    {
+      command: 'assertEval',
+      target: 'document.querySelector("#attribute-type-test-menu li:last-child[aria-selected=true]") != null',
+      value: 'true',
+    },
+    // Check that the scroll position did not change.
+    {
+      command: 'assertEval',
+      target: 'document.querySelector("#attribute-type-test-menu").scrollTop',
+      value: '0',
+    },
+    {
+      command: 'type',
+      target: 'id=attribute-type-test-input',
+      value: '',
+    },
+    // Close the menu, open it with the down arrow key, and then move to the
+    // last item with the up arrow key.
+    {
+      command: 'pause',
+      target: '100',
+      value: '',
+    },
+    {
+      command: 'sendKeys',
+      target: 'id=attribute-type-test-input',
+      value: '${KEY_ESC}',
+    },
+    {
+      command: 'pause',
+      target: '100',
+      value: '',
+    },
+    {
+      command: 'sendKeys',
+      target: 'id=attribute-type-test-input',
+      value: '${KEY_DOWN}',
+    },
+    {
+      command: 'pause',
+      target: '100',
+      value: '',
+    },
+    {
+      command: 'sendKeys',
+      target: 'id=attribute-type-test-input',
+      value: '${KEY_UP}',
+    },
+    // Check that the scroll position changed.
+    {
+      command: 'assertEval',
+      target: 'document.querySelector("#attribute-type-test-menu").scrollTop > 0',
+      value: 'true',
+    },
+    // Close the menu, open it with the down arrow key, and check that the
+    // scroll position is back at the top.
+    {
+      command: 'pause',
+      target: '100',
+      value: '',
+    },
+    {
+      command: 'sendKeys',
+      target: 'id=attribute-type-test-input',
+      value: '${KEY_ESC}',
+    },
+    {
+      command: 'pause',
+      target: '100',
+      value: '',
+    },
+    {
+      command: 'sendKeys',
+      target: 'id=attribute-type-test-input',
+      value: '${KEY_DOWN}',
+    },
+    {
+      command: 'assertEval',
+      target: 'document.querySelector("#attribute-type-test-menu").scrollTop',
+      value: '0',
+    },
+    // End of test for MBS-12936.
   ],
 }