lib: Make SimpleSelect and CheckboxSelect generic in the option value…

… type The "forwarded ref" machinery has been removed. We probably don't need it, I don't know how to make it work with the generics, and it is deprecated in React 19.
cockpit-project · Dec 19, 2024 · 3b9e6cc · 3b9e6cc
1 parent 7b9fc65
commit 3b9e6cc
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 40 deletions.
diff --git a/pkg/lib/cockpit-components-checkbox-select.tsx b/pkg/lib/cockpit-components-checkbox-select.tsx
@@ -64,22 +64,20 @@ import {
   SelectProps
 } from '@patternfly/react-core';
 
-export interface CheckboxSelectOption extends Omit<SelectOptionProps, 'content'> {
+export interface CheckboxSelectOption<T> extends Omit<SelectOptionProps, 'content'> {
   /** Content of the select option. */
   content: React.ReactNode;
   /** Value of the select option. */
-  value: unknown;
+  value: T;
 }
 
-export interface CheckboxSelectProps extends Omit<SelectProps, 'toggle' | 'onSelect'> {
-  /** @hide Forwarded ref */
-  innerRef?: React.Ref<any>;
+export interface CheckboxSelectProps<T> extends Omit<SelectProps, 'toggle' | 'onSelect'> {
   /** Options of the select. */
-  options?: CheckboxSelectOption[];
+  options?: CheckboxSelectOption<T>[];
   /** Currently checked options */
-  selected: unknown[];
+  selected: T[];
   /** Callback triggered when checking or unchecking an option. */
-  onSelect: (value: unknown, checked: boolean) => void;
+  onSelect: (value: T, checked: boolean) => void;
   /** Callback triggered when the select opens or closes. */
   onToggle?: (nextIsOpen: boolean) => void;
   /** Flag indicating the select should be disabled. */
@@ -94,8 +92,7 @@ export interface CheckboxSelectProps extends Omit<SelectProps, 'toggle' | 'onSel
   noBadge?: boolean,
 }
 
-const CheckboxSelectBase: React.FunctionComponent<CheckboxSelectProps> = ({
-  innerRef,
+export function CheckboxSelect<T>({
   options,
   selected,
   isDisabled = false,
@@ -106,7 +103,7 @@ const CheckboxSelectBase: React.FunctionComponent<CheckboxSelectProps> = ({
   toggleProps,
   noBadge = false,
   ...props
-}: CheckboxSelectProps) => {
+}: CheckboxSelectProps<T>) {
   const [isOpen, setIsOpen] = React.useState(false);
 
   const checkboxSelectOptions = options?.map((option) => {
@@ -125,9 +122,10 @@ const CheckboxSelectBase: React.FunctionComponent<CheckboxSelectProps> = ({
     setIsOpen(!isOpen);
   };
 
-  const _onSelect = (event: React.MouseEvent<Element, MouseEvent> | undefined, value: string | number | undefined) => {
-      if (value && event)
-          onSelect(value, (event.target as HTMLInputElement).checked);
+  const _onSelect = (event: React.MouseEvent<Element, MouseEvent> | undefined, value: T | undefined) => {
+    if (value && event) {
+      onSelect(value, (event.target as HTMLInputElement).checked);
+    }
   };
 
   const toggle = (toggleRef: React.Ref<MenuToggleElement>) => (
@@ -155,21 +153,17 @@ const CheckboxSelectBase: React.FunctionComponent<CheckboxSelectProps> = ({
     <Select
       isOpen={isOpen}
       selected={selected}
+      // @ts-expect-error https://github.com/patternfly/patternfly-react/issues/11361
       onSelect={_onSelect}
       onOpenChange={(isOpen) => {
         onToggle && onToggle(isOpen);
         setIsOpen(isOpen);
       }}
       toggle={toggle}
-      ref={innerRef}
       role="menu"
       {...props}
     >
       <SelectList>{checkboxSelectOptions}</SelectList>
     </Select>
   );
 };
-
-export const CheckboxSelect = React.forwardRef((props: CheckboxSelectProps, ref: React.Ref<any>) => (
-  <CheckboxSelectBase {...props} innerRef={ref} />
-));
diff --git a/pkg/lib/cockpit-components-simple-select.tsx b/pkg/lib/cockpit-components-simple-select.tsx
@@ -69,27 +69,25 @@ export interface SimpleSelectDividerOption {
   key: string | number;
 };
 
-export interface SimpleSelectMenuOption extends Omit<SelectOptionProps, 'content'> {
+export interface SimpleSelectMenuOption<T> extends Omit<SelectOptionProps, 'content'> {
   decorator?: undefined;
 
   /** Content of the select option. */
   content: React.ReactNode;
   /** Value of the select option. */
-  value: unknown;
+  value: T;
 }
 
-export type SimpleSelectOption = SimpleSelectMenuOption |
-                                 SimpleSelectDividerOption;
+export type SimpleSelectOption<T> = SimpleSelectMenuOption<T> |
+                                    SimpleSelectDividerOption;
 
-export interface SimpleSelectProps extends Omit<SelectProps, 'toggle' | 'onSelect'> {
-  /** @hide Forwarded ref */
-  innerRef?: React.Ref<any>;
+export interface SimpleSelectProps<T> extends Omit<SelectProps, 'toggle' | 'onSelect'> {
   /** Initial options of the select. */
-  options: SimpleSelectOption[];
+  options: SimpleSelectOption<T>[];
   /** Selected option */
   selected: unknown;
   /** Callback triggered on selection. */
-  onSelect: (selection: unknown) => void;
+  onSelect: (selection: T) => void;
   /** Callback triggered when the select opens or closes. */
   onToggle?: (nextIsOpen: boolean) => void;
   /** Flag indicating the select should be disabled. */
@@ -104,8 +102,7 @@ export interface SimpleSelectProps extends Omit<SelectProps, 'toggle' | 'onSelec
   toggleProps?: MenuToggleProps;
 }
 
-const SimpleSelectBase: React.FunctionComponent<SimpleSelectProps> = ({
-  innerRef,
+export function SimpleSelect<T>({
   options,
   selected,
   isDisabled = false,
@@ -116,10 +113,10 @@ const SimpleSelectBase: React.FunctionComponent<SimpleSelectProps> = ({
   toggleProps,
   placeholder = '',
   ...props
-}: SimpleSelectProps) => {
+}: SimpleSelectProps<T>) {
   const [isOpen, setIsOpen] = React.useState(false);
 
-  const simpleSelectOptions = options.map((option, index) => {
+  const simpleSelectOptions = options.map(option => {
     if (option.decorator == "divider")
       return <Divider key={option.key} component="li" />;
     const { content, value, key, ...props } = option;
@@ -135,9 +132,10 @@ const SimpleSelectBase: React.FunctionComponent<SimpleSelectProps> = ({
     setIsOpen(!isOpen);
   };
 
-  const _onSelect = (_event: React.MouseEvent<Element, MouseEvent> | undefined, value: unknown) => {
-    if (value)
+  const _onSelect = (_event: React.MouseEvent<Element, MouseEvent> | undefined, value: T | undefined) => {
+    if (value) {
       onSelect(value);
+    }
     onToggle && onToggle(true);
     setIsOpen(false);
   };
@@ -146,7 +144,7 @@ const SimpleSelectBase: React.FunctionComponent<SimpleSelectProps> = ({
   if (toggleContent)
     content = toggleContent;
   else if (selected)
-    content = options.find((o): o is SimpleSelectMenuOption => !o.decorator && o.value == selected)?.content;
+    content = options.find((o): o is SimpleSelectMenuOption<T> => !o.decorator && o.value == selected)?.content;
 
   const toggle = (toggleRef: React.Ref<MenuToggleElement>) => (
     <MenuToggle
@@ -169,23 +167,19 @@ const SimpleSelectBase: React.FunctionComponent<SimpleSelectProps> = ({
     <Select
       isOpen={isOpen}
       selected={selected}
+      // @ts-expect-error https://github.com/patternfly/patternfly-react/issues/11361
       onSelect={_onSelect}
       onOpenChange={(isOpen) => {
         onToggle && onToggle(isOpen);
         setIsOpen(isOpen);
       }}
       toggle={toggle}
       shouldFocusToggleOnSelect
-      ref={innerRef}
       {...props}
     >
       <SelectList>{simpleSelectOptions}</SelectList>
     </Select>
   );
 };
 
-export const SimpleSelect = React.forwardRef((props: SimpleSelectProps, ref: React.Ref<any>) => (
-  <SimpleSelectBase {...props} innerRef={ref} />
-));
-
 SimpleSelect.displayName = 'SimpleSelect';