From f34626162b9bd49a50ae5e143b9885ff8128d1d6 Mon Sep 17 00:00:00 2001
From: Jeffrey Phillips <jephilli@redhat.com>
Date: Thu, 29 Aug 2024 13:00:09 -0400
Subject: [PATCH] Fix for select components causing scroll container to jump

---
 frontend/src/components/MultiSelection.tsx    | 109 ++++++++-------
 frontend/src/components/SimpleSelect.tsx      | 117 +++++++++-------
 frontend/src/components/TypeaheadSelect.tsx   |  59 ++++----
 .../fields/DropdownFormField.tsx              | 131 ++++++++++--------
 .../src/utilities/WithScrollContainer.tsx     |  47 +++++++
 5 files changed, 274 insertions(+), 189 deletions(-)
 create mode 100644 frontend/src/utilities/WithScrollContainer.tsx

diff --git a/frontend/src/components/MultiSelection.tsx b/frontend/src/components/MultiSelection.tsx
index 0f141bf77d..5a1b7829ac 100644
--- a/frontend/src/components/MultiSelection.tsx
+++ b/frontend/src/components/MultiSelection.tsx
@@ -15,8 +15,10 @@ import {
   HelperTextItem,
   SelectGroup,
   Divider,
+  SelectPopperProps,
 } from '@patternfly/react-core';
-import { TimesIcon } from '@patternfly/react-icons/dist/esm/icons/times-icon';
+import { TimesIcon } from '@patternfly/react-icons';
+import { WithScrollContainer } from '~/utilities/WithScrollContainer';
 
 export type SelectionOptions = {
   id: number | string;
@@ -42,6 +44,7 @@ type MultiSelectionProps = {
   selectionRequired?: boolean;
   noSelectedOptionsMessage?: string;
   toggleTestId?: string;
+  popperProps?: SelectPopperProps;
 };
 
 export const MultiSelection: React.FC<MultiSelectionProps> = ({
@@ -56,6 +59,7 @@ export const MultiSelection: React.FC<MultiSelectionProps> = ({
   toggleTestId,
   selectionRequired,
   noSelectedOptionsMessage = 'One or more options must be selected',
+  popperProps = {},
 }) => {
   const [isOpen, setIsOpen] = React.useState(false);
   const [inputValue, setInputValue] = React.useState<string>('');
@@ -255,28 +259,50 @@ export const MultiSelection: React.FC<MultiSelectionProps> = ({
   );
 
   return (
-    <>
-      <Select
-        id={id}
-        isOpen={isOpen}
-        selected={selected}
-        onSelect={(ev, selection) => {
-          const selectedOption = allOptions.find((option) => option.id === selection);
-          onSelect(selectedOption);
-        }}
-        onOpenChange={() => setOpen(false)}
-        toggle={toggle}
-      >
-        {visibleOptions.length === 0 && inputValue ? (
-          <SelectList isAriaMultiselectable>
-            <SelectOption isDisabled>No results found</SelectOption>
-          </SelectList>
-        ) : null}
-        {selectGroups.map((g, index) => (
-          <>
-            <SelectGroup label={g.name} key={g.id}>
+    <WithScrollContainer>
+      {(scrollContainer) => (
+        <>
+          <Select
+            id={id}
+            isOpen={isOpen}
+            selected={selected}
+            onSelect={(ev, selection) => {
+              const selectedOption = allOptions.find((option) => option.id === selection);
+              onSelect(selectedOption);
+            }}
+            onOpenChange={() => setOpen(false)}
+            toggle={toggle}
+            popperProps={{ appendTo: scrollContainer, ...popperProps }}
+          >
+            {visibleOptions.length === 0 && inputValue ? (
               <SelectList isAriaMultiselectable>
-                {g.values.map((option) => (
+                <SelectOption isDisabled>No results found</SelectOption>
+              </SelectList>
+            ) : null}
+            {selectGroups.map((g, index) => (
+              <>
+                <SelectGroup label={g.name} key={g.id}>
+                  <SelectList isAriaMultiselectable>
+                    {g.values.map((option) => (
+                      <SelectOption
+                        key={option.name}
+                        isFocused={focusedItemIndex === option.index}
+                        id={`select-multi-typeahead-${option.name.replace(' ', '-')}`}
+                        value={option.id}
+                        ref={null}
+                        isSelected={option.selected}
+                      >
+                        {option.name}
+                      </SelectOption>
+                    ))}
+                  </SelectList>
+                </SelectGroup>
+                {index < selectGroups.length - 1 || selectOptions.length ? <Divider /> : null}
+              </>
+            ))}
+            {selectOptions.length ? (
+              <SelectList isAriaMultiselectable>
+                {selectOptions.map((option) => (
                   <SelectOption
                     key={option.name}
                     isFocused={focusedItemIndex === option.index}
@@ -289,34 +315,17 @@ export const MultiSelection: React.FC<MultiSelectionProps> = ({
                   </SelectOption>
                 ))}
               </SelectList>
-            </SelectGroup>
-            {index < selectGroups.length - 1 || selectOptions.length ? <Divider /> : null}
-          </>
-        ))}
-        {selectOptions.length ? (
-          <SelectList isAriaMultiselectable>
-            {selectOptions.map((option) => (
-              <SelectOption
-                key={option.name}
-                isFocused={focusedItemIndex === option.index}
-                id={`select-multi-typeahead-${option.name.replace(' ', '-')}`}
-                value={option.id}
-                ref={null}
-                isSelected={option.selected}
-              >
-                {option.name}
-              </SelectOption>
-            ))}
-          </SelectList>
-        ) : null}
-      </Select>
-      {noSelectedItems && selectionRequired && (
-        <HelperText isLiveRegion>
-          <HelperTextItem variant="error" hasIcon data-testid="group-selection-error-text">
-            {noSelectedOptionsMessage}
-          </HelperTextItem>
-        </HelperText>
+            ) : null}
+          </Select>
+          {noSelectedItems && selectionRequired && (
+            <HelperText isLiveRegion>
+              <HelperTextItem variant="error" hasIcon data-testid="group-selection-error-text">
+                {noSelectedOptionsMessage}
+              </HelperTextItem>
+            </HelperText>
+          )}
+        </>
       )}
-    </>
+    </WithScrollContainer>
   );
 };
diff --git a/frontend/src/components/SimpleSelect.tsx b/frontend/src/components/SimpleSelect.tsx
index 27960895ff..a1d7b62e06 100644
--- a/frontend/src/components/SimpleSelect.tsx
+++ b/frontend/src/components/SimpleSelect.tsx
@@ -9,6 +9,7 @@ import {
   Divider,
   MenuToggleProps,
 } from '@patternfly/react-core';
+import { WithScrollContainer } from '~/utilities/WithScrollContainer';
 
 import './SimpleSelect.scss';
 
@@ -56,6 +57,7 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
   icon,
   dataTestId,
   toggleProps,
+  popperProps = {},
   ...props
 }) => {
   const [open, setOpen] = React.useState(false);
@@ -70,41 +72,66 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
   const selectedLabel = selectedOption?.label ?? placeholder;
 
   return (
-    <Select
-      {...props}
-      isOpen={open}
-      selected={value || toggleLabel}
-      onSelect={(e, selectValue) => {
-        onChange(
-          String(selectValue),
-          !!selectValue && (findOptionForKey(String(selectValue))?.isPlaceholder ?? false),
-        );
-        setOpen(false);
-      }}
-      onOpenChange={setOpen}
-      toggle={(toggleRef) => (
-        <MenuToggle
-          ref={toggleRef}
-          data-testid={dataTestId}
-          aria-label="Options menu"
-          onClick={() => setOpen(!open)}
-          icon={icon}
-          isExpanded={open}
-          isDisabled={isDisabled}
-          isFullWidth={isFullWidth}
-          {...toggleProps}
+    <WithScrollContainer>
+      {(scrollContainer) => (
+        <Select
+          {...props}
+          popperProps={{ appendTo: scrollContainer, ...popperProps }}
+          isOpen={open}
+          selected={value || toggleLabel}
+          onSelect={(e, selectValue) => {
+            onChange(
+              String(selectValue),
+              !!selectValue && (findOptionForKey(String(selectValue))?.isPlaceholder ?? false),
+            );
+            setOpen(false);
+          }}
+          onOpenChange={setOpen}
+          toggle={(toggleRef) => (
+            <MenuToggle
+              ref={toggleRef}
+              data-testid={dataTestId}
+              aria-label="Options menu"
+              onClick={() => setOpen(!open)}
+              icon={icon}
+              isExpanded={open}
+              isDisabled={isDisabled}
+              isFullWidth={isFullWidth}
+              {...toggleProps}
+            >
+              {toggleLabel || (
+                <Truncate content={selectedLabel} className="truncate-no-min-width" />
+              )}
+            </MenuToggle>
+          )}
+          shouldFocusToggleOnSelect
         >
-          {toggleLabel || <Truncate content={selectedLabel} className="truncate-no-min-width" />}
-        </MenuToggle>
-      )}
-      shouldFocusToggleOnSelect
-    >
-      {groupedOptions?.map((group, index) => (
-        <>
-          {index > 0 ? <Divider /> : null}
-          <SelectGroup key={group.key} label={group.label}>
+          {groupedOptions?.map((group, index) => (
+            <>
+              {index > 0 ? <Divider /> : null}
+              <SelectGroup key={group.key} label={group.label}>
+                <SelectList>
+                  {group.options.map(
+                    ({ key, label, dropdownLabel, description, isDisabled: optionDisabled }) => (
+                      <SelectOption
+                        key={key}
+                        value={key}
+                        description={description}
+                        isDisabled={optionDisabled}
+                        data-testid={key}
+                      >
+                        {dropdownLabel || label}
+                      </SelectOption>
+                    ),
+                  )}
+                </SelectList>
+              </SelectGroup>
+            </>
+          )) ?? null}
+          {options?.length ? (
             <SelectList>
-              {group.options.map(
+              {groupedOptions?.length ? <Divider /> : null}
+              {options.map(
                 ({ key, label, dropdownLabel, description, isDisabled: optionDisabled }) => (
                   <SelectOption
                     key={key}
@@ -118,26 +145,10 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
                 ),
               )}
             </SelectList>
-          </SelectGroup>
-        </>
-      )) ?? null}
-      {options?.length ? (
-        <SelectList>
-          {groupedOptions?.length ? <Divider /> : null}
-          {options.map(({ key, label, dropdownLabel, description, isDisabled: optionDisabled }) => (
-            <SelectOption
-              key={key}
-              value={key}
-              description={description}
-              isDisabled={optionDisabled}
-              data-testid={key}
-            >
-              {dropdownLabel || label}
-            </SelectOption>
-          ))}
-        </SelectList>
-      ) : null}
-    </Select>
+          ) : null}
+        </Select>
+      )}
+    </WithScrollContainer>
   );
 };
 
diff --git a/frontend/src/components/TypeaheadSelect.tsx b/frontend/src/components/TypeaheadSelect.tsx
index 28932e5436..daccd4dbe5 100644
--- a/frontend/src/components/TypeaheadSelect.tsx
+++ b/frontend/src/components/TypeaheadSelect.tsx
@@ -12,8 +12,10 @@ import {
   Button,
   MenuToggleProps,
   SelectProps,
+  SelectPopperProps,
 } from '@patternfly/react-core';
 import { TimesIcon } from '@patternfly/react-icons';
+import { WithScrollContainer } from '~/utilities/WithScrollContainer';
 
 export interface TypeaheadSelectOption extends Omit<SelectOptionProps, 'content' | 'isSelected'> {
   /** Content of the select option. */
@@ -66,6 +68,7 @@ export interface TypeaheadSelectProps extends Omit<SelectProps, 'toggle' | 'onSe
   toggleWidth?: string;
   /** Additional props passed to the toggle. */
   toggleProps?: MenuToggleProps;
+  popperProps?: SelectPopperProps;
 }
 
 const defaultNoOptionsFoundMessage = (filter: string) => `No results found for "${filter}"`;
@@ -92,6 +95,7 @@ const TypeaheadSelect: React.FunctionComponent<TypeaheadSelectProps> = ({
   isDisabled,
   toggleWidth,
   toggleProps,
+  popperProps = {},
   ...props
 }: TypeaheadSelectProps) => {
   const [isOpen, setIsOpen] = React.useState(false);
@@ -395,31 +399,36 @@ const TypeaheadSelect: React.FunctionComponent<TypeaheadSelectProps> = ({
   );
 
   return (
-    <Select
-      isOpen={isOpen}
-      selected={selected}
-      onSelect={handleSelect}
-      onOpenChange={(open) => !open && closeMenu()}
-      toggle={toggle}
-      ref={innerRef}
-      {...props}
-    >
-      <SelectList>
-        {filteredSelections.map((option, index) => {
-          const { content, value, ...optionProps } = option;
-          return (
-            <SelectOption
-              key={value}
-              value={value}
-              isFocused={focusedItemIndex === index}
-              {...optionProps}
-            >
-              {content}
-            </SelectOption>
-          );
-        })}
-      </SelectList>
-    </Select>
+    <WithScrollContainer>
+      {(scrollContainer) => (
+        <Select
+          isOpen={isOpen}
+          selected={selected}
+          onSelect={handleSelect}
+          onOpenChange={(open) => !open && closeMenu()}
+          toggle={toggle}
+          ref={innerRef}
+          popperProps={{ appendTo: scrollContainer, ...popperProps }}
+          {...props}
+        >
+          <SelectList>
+            {filteredSelections.map((option, index) => {
+              const { content, value, ...optionProps } = option;
+              return (
+                <SelectOption
+                  key={value}
+                  value={value}
+                  isFocused={focusedItemIndex === index}
+                  {...optionProps}
+                >
+                  {content}
+                </SelectOption>
+              );
+            })}
+          </SelectList>
+        </Select>
+      )}
+    </WithScrollContainer>
   );
 };
 
diff --git a/frontend/src/concepts/connectionTypes/fields/DropdownFormField.tsx b/frontend/src/concepts/connectionTypes/fields/DropdownFormField.tsx
index f720dc985b..785f0cf4ee 100644
--- a/frontend/src/concepts/connectionTypes/fields/DropdownFormField.tsx
+++ b/frontend/src/concepts/connectionTypes/fields/DropdownFormField.tsx
@@ -3,6 +3,7 @@ import { Badge, MenuToggle, Select, SelectList, SelectOption } from '@patternfly
 import { DropdownField } from '~/concepts/connectionTypes/types';
 import { FieldProps } from '~/concepts/connectionTypes/fields/types';
 import DefaultValueTextRenderer from '~/concepts/connectionTypes/fields/DefaultValueTextRenderer';
+import { WithScrollContainer } from '~/utilities/WithScrollContainer';
 
 const DropdownFormField: React.FC<FieldProps<DropdownField>> = ({
   id,
@@ -16,73 +17,81 @@ const DropdownFormField: React.FC<FieldProps<DropdownField>> = ({
   const [isOpen, setIsOpen] = React.useState(false);
   const isMulti = field.properties.variant === 'multi';
   const selected = isPreview ? field.properties.defaultValue : value;
+
   return (
     <DefaultValueTextRenderer id={id} field={field} mode={mode}>
-      <Select
-        isOpen={isOpen}
-        shouldFocusToggleOnSelect
-        selected={selected}
-        onSelect={
-          isPreview || !onChange
-            ? undefined
-            : (_e, v) => {
-                if (isMulti) {
-                  if (selected?.includes(String(v))) {
-                    onChange(selected.filter((s) => s !== v));
-                  } else {
-                    onChange([...(selected || []), String(v)]);
+      <WithScrollContainer>
+        {(scrollContainer) => (
+          <Select
+            isOpen={isOpen}
+            shouldFocusToggleOnSelect
+            selected={selected}
+            onSelect={
+              isPreview || !onChange
+                ? undefined
+                : (_e, v) => {
+                    if (isMulti) {
+                      if (selected?.includes(String(v))) {
+                        onChange(selected.filter((s) => s !== v));
+                      } else {
+                        onChange([...(selected || []), String(v)]);
+                      }
+                    } else {
+                      onChange([String(v)]);
+                      setIsOpen(false);
+                    }
                   }
-                } else {
-                  onChange([String(v)]);
-                  setIsOpen(false);
-                }
-              }
-        }
-        onOpenChange={(open) => setIsOpen(open)}
-        toggle={(toggleRef) => (
-          <MenuToggle
-            ref={toggleRef}
-            id={id}
-            data-testid={dataTestId}
-            isFullWidth
-            onClick={() => {
-              setIsOpen((open) => !open);
+            }
+            onOpenChange={(open) => setIsOpen(open)}
+            popperProps={{
+              appendTo: scrollContainer,
             }}
-            isExpanded={isOpen}
-          >
-            {isMulti ? (
-              <>
-                Select {field.name}{' '}
-                <Badge>
-                  {(isPreview ? field.properties.defaultValue?.length : value?.length) ?? 0}{' '}
-                  selected
-                </Badge>
-              </>
-            ) : (
-              (isPreview
-                ? field.properties.items?.find(
-                    (i) => i.value === field.properties.defaultValue?.[0],
-                  )?.label
-                : field.properties.items?.find((i) => value?.includes(i.value))?.label) ||
-              `Select ${field.name}`
+            toggle={(toggleRef) => (
+              <MenuToggle
+                ref={toggleRef}
+                id={id}
+                data-testid={dataTestId}
+                isFullWidth
+                onClick={() => {
+                  setIsOpen((open) => !open);
+                }}
+                isExpanded={isOpen}
+              >
+                {isMulti ? (
+                  <>
+                    Select {field.name}{' '}
+                    <Badge>
+                      {(isPreview ? field.properties.defaultValue?.length : value?.length) ?? 0}{' '}
+                      selected
+                    </Badge>
+                  </>
+                ) : (
+                  (isPreview
+                    ? field.properties.items?.find(
+                        (i) => i.value === field.properties.defaultValue?.[0],
+                      )?.label
+                    : field.properties.items?.find((i) => value?.includes(i.value))?.label) ||
+                  `Select ${field.name}`
+                )}
+              </MenuToggle>
             )}
-          </MenuToggle>
+          >
+            <SelectList>
+              {field.properties.items?.map((i) => (
+                <SelectOption
+                  value={i.value}
+                  key={i.value}
+                  hasCheckbox={isMulti}
+                  selected={selected?.includes(i.value)}
+                  description={`Value: ${i.value}`}
+                >
+                  {i.label}
+                </SelectOption>
+              ))}
+            </SelectList>
+          </Select>
         )}
-      >
-        <SelectList>
-          {field.properties.items?.map((i) => (
-            <SelectOption
-              value={i.value}
-              key={i.value}
-              hasCheckbox={isMulti}
-              selected={selected?.includes(i.value)}
-              description={`Value: ${i.value}`}
-            >
-              {i.label}
-            </SelectOption>
-          ))}
-        </SelectList>
-      </Select>
+      </WithScrollContainer>
     </DefaultValueTextRenderer>
   );
 };
diff --git a/frontend/src/utilities/WithScrollContainer.tsx b/frontend/src/utilities/WithScrollContainer.tsx
new file mode 100644
index 0000000000..5a5d91eec9
--- /dev/null
+++ b/frontend/src/utilities/WithScrollContainer.tsx
@@ -0,0 +1,47 @@
+import * as React from 'react';
+
+const isHTMLElement = (n: Node): n is HTMLElement => n.nodeType === Node.ELEMENT_NODE;
+
+export const getParentScrollableElement = (node: HTMLElement | null): HTMLElement | undefined => {
+  let parentNode: Node | null = node;
+  while (parentNode) {
+    if (isHTMLElement(parentNode)) {
+      let { overflow } = parentNode.style;
+      if (!overflow.includes('scroll') && !overflow.includes('auto')) {
+        overflow = window.getComputedStyle(parentNode).overflow;
+      }
+      if (overflow.includes('scroll') || overflow.includes('auto')) {
+        return parentNode;
+      }
+    }
+    parentNode = parentNode.parentNode;
+  }
+  return undefined;
+};
+
+type WithScrollContainerProps = {
+  children: (scrollContainer: HTMLElement | 'inline') => React.ReactElement | null;
+};
+
+export const WithScrollContainer: React.FC<WithScrollContainerProps> = ({ children }) => {
+  const [scrollContainer, setScrollContainer] = React.useState<HTMLElement | null>();
+  const ref = React.useCallback((node: HTMLElement | null) => {
+    if (node) {
+      setScrollContainer(getParentScrollableElement(node));
+    }
+  }, []);
+  return scrollContainer ? children(scrollContainer) : <span ref={ref}>{children('inline')}</span>;
+};
+
+export const useScrollContainer = (): [HTMLElement | undefined, (node: HTMLElement) => void] => {
+  const [scrollContainer, setScrollContainer] = React.useState<HTMLElement | undefined>();
+  const elementRef = React.useCallback((node: HTMLElement | null) => {
+    if (node === null) {
+      setScrollContainer(undefined);
+    }
+    if (node) {
+      setScrollContainer(getParentScrollableElement(node));
+    }
+  }, []);
+  return [scrollContainer, elementRef];
+};