fix: ModalPicker button onClick propagation

When the button in ModalPicker components is clicked, the event is propagated, causing parent elements onClick to trigger as well. Added event preventDefault to ModalPicker components.
42BV · Feb 1, 2024 · be2b14f · be2b14f
1 parent 8ad8f7d
commit be2b14f
Show file tree

Hide file tree

Showing 4 changed files with 137 additions and 121 deletions.
diff --git a/src/core/Button/Button.tsx b/src/core/Button/Button.tsx
@@ -28,7 +28,7 @@ export type Props = {
   /**
    *  Optional callback for what needs to happen when the button is clicked.
    */
-  onClick?: (event: React.MouseEvent<HTMLElement>) => any;
+  onClick?: React.MouseEventHandler<HTMLButtonElement>;
 
   /**
    * Whether the action you are performing is currently in progress.
@@ -124,15 +124,15 @@ export function Button({
 }: Props) {
   const showSpinner = useShowSpinner(!!inProgress);
 
-  function handleOnClick(event: React.MouseEvent<HTMLElement>) {
+  const handleOnClick: React.MouseEventHandler<HTMLButtonElement> = (event) => {
     if (!onClick) {
       return;
     }
 
     if (!inProgress) {
       onClick(event);
     }
-  }
+  };
 
   // If there are children it will look like a button.
   if (children) {

diff --git a/src/form/ModalPicker/ModalPickerOpener/ModalPickerOpener.tsx b/src/form/ModalPicker/ModalPickerOpener/ModalPickerOpener.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { MouseEventHandler } from 'react';
 import { Button } from 'reactstrap';
 import { ModalPickerButtonAlignment } from '../types';
 import classNames from 'classnames';
@@ -16,7 +16,7 @@ type BaseProps = {
   /**
    * Function to open the modal, called when the button is clicked.
    */
-  openModal: () => void;
+  openModal: MouseEventHandler<HTMLButtonElement>;
 
   /**
    * The label to display on the button.
@@ -60,18 +60,16 @@ type Props<T> =
   | ModalPickerSingleOpenerProps<T>
   | ModalPickerMultipleOpenerProps<T>;
 
-export function ModalPickerOpener<T>(props: Props<T>) {
-  const {
-    openModal,
-    label,
-    icon,
-    alignButton,
-    onClear,
-    text = {},
-    renderValue,
-    value
-  } = props;
-
+export function ModalPickerOpener<T>({
+  openModal,
+  label,
+  icon,
+  alignButton,
+  onClear,
+  text = {},
+  renderValue,
+  value
+}: Props<T>) {
   const hasValue = !!value;
 
   const wrapperClassName = classNames('d-flex', 'align-items-center', {

diff --git a/src/form/ModalPicker/multiple/ModalPickerMultiple.tsx b/src/form/ModalPicker/multiple/ModalPickerMultiple.tsx
@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { MouseEventHandler, useState } from 'react';
 import { Col, FormGroup, Input, Label, Row } from 'reactstrap';
 
 import { withJarb } from '../../withJarb/withJarb';
@@ -176,7 +176,9 @@ export function ModalPickerMultiple<T>(props: Props<T>) {
     setIsOpen(false);
   }
 
-  function openModal() {
+  const openModal: MouseEventHandler<HTMLButtonElement> = (event) => {
+    event.preventDefault();
+
     // Always copy the `value` so the `selected` is a fresh array.
     // Otherwise, the selection will be the same as the value, which
     // causes values to be committed and the cancel button will not
@@ -188,7 +190,7 @@ export function ModalPickerMultiple<T>(props: Props<T>) {
     setQuery('');
     setPageNumber(1);
     setUserHasSearched(false);
-  }
+  };
 
   function optionClicked(option: T, isSelected: boolean) {
     // Always copy the `value` so the `selected` is a fresh array.

diff --git a/src/form/ModalPicker/single/ModalPickerSingle.tsx b/src/form/ModalPicker/single/ModalPickerSingle.tsx
@@ -1,94 +1,105 @@
-import React, { useState } from 'react';
+import React, { MouseEventHandler, useState } from 'react';
 import { FormGroup, Input, Label } from 'reactstrap';
-import { FieldCompatibleWithPredeterminedOptions, getKeyForOption, isOptionSelected } from '../../option';
+import {
+  FieldCompatibleWithPredeterminedOptions,
+  getKeyForOption,
+  isOptionSelected
+} from '../../option';
 import { FieldCompatible } from '../../types';
 import { useOptions } from '../../useOptions';
 import { alwaysTrue } from '../../utils';
 import { withJarb } from '../../withJarb/withJarb';
 import { ModalPicker, Text } from '../ModalPicker';
 import { ModalPickerOpener } from '../ModalPickerOpener/ModalPickerOpener';
 import { ModalPickerValueTruncator } from '../ModalPickerValueTruncator/ModalPickerValueTruncator';
-import { ModalPickerAddButtonCallback, ModalPickerAddButtonOptions, ModalPickerButtonAlignment, ModalPickerRenderOptions } from '../types';
+import {
+  ModalPickerAddButtonCallback,
+  ModalPickerAddButtonOptions,
+  ModalPickerButtonAlignment,
+  ModalPickerRenderOptions
+} from '../types';
 import { IconType } from '../../../core/Icon';
 import { withField } from '../../withField/withField';
 
 export type ModalPickerSingleRenderValue<T> = (value?: T) => React.ReactNode;
 
-type Props<T> = Omit<FieldCompatible<T, T | undefined>,
-  'placeholder' | 'valid'> &
+type Props<T> = Omit<
+  FieldCompatible<T, T | undefined>,
+  'placeholder' | 'valid'
+> &
   FieldCompatibleWithPredeterminedOptions<T> & {
-  /**
-   * The placeholder of the form element.
-   */
-  placeholder: string;
-
-  /**
-   * Optionally the icon to display on the button to open the modal picker.
-   */
-  icon?: IconType;
-
-  /**
-   * Optionally whether the user can search.
-   * Defaults to `true`.
-   */
-  canSearch?: boolean;
-
-  /**
-   * Optionally specify the number of options to show / fetch per
-   * page in the modal.
-   *
-   * When `options` is an array, it will determine how many options
-   * will be displayed per page.
-   *
-   * When `options` is a fetcher, it will determine how many options
-   * are requested through the fetcher as the `page` parameter.
-   * This means that when you set the pageSize to `100` that
-   * `100` items are fetched from the back-end. Beware of
-   * performance issues when setting the value too high.
-   *
-   * Beware that setting the page size too high will cause the UX
-   * to deteriorate on smaller screens!
-   *
-   * Defaults to `10`.
-   */
-  pageSize?: number;
-
-  /**
-   * Optionally an add button to display in the Modal. Can
-   * be used to dynamically add an option which was not there
-   * before.
-   */
-  addButton?: ModalPickerAddButtonOptions<T>;
-
-  /**
-   * Optionally the position the button should be aligned to
-   * within its container.
-   */
-  alignButton?: ModalPickerButtonAlignment;
-
-  /**
-   * Optionally callback to display the selected item.
-   */
-  renderValue?: ModalPickerSingleRenderValue<T>;
-
-  /**
-   * Callback to customize display of options.
-   */
-  renderOptions?: ModalPickerRenderOptions<T>;
-
-  /**
-   * Whether to show a "clear" button.
-   *
-   * Defaults to `true`
-   */
-  canClear?: boolean;
-
-  /**
-   * Optionally customized text within the component.
-   * This text should already be translated.
-   */
-  text?: Text;
-};
+    /**
+     * The placeholder of the form element.
+     */
+    placeholder: string;
+
+    /**
+     * Optionally the icon to display on the button to open the modal picker.
+     */
+    icon?: IconType;
+
+    /**
+     * Optionally whether the user can search.
+     * Defaults to `true`.
+     */
+    canSearch?: boolean;
+
+    /**
+     * Optionally specify the number of options to show / fetch per
+     * page in the modal.
+     *
+     * When `options` is an array, it will determine how many options
+     * will be displayed per page.
+     *
+     * When `options` is a fetcher, it will determine how many options
+     * are requested through the fetcher as the `page` parameter.
+     * This means that when you set the pageSize to `100` that
+     * `100` items are fetched from the back-end. Beware of
+     * performance issues when setting the value too high.
+     *
+     * Beware that setting the page size too high will cause the UX
+     * to deteriorate on smaller screens!
+     *
+     * Defaults to `10`.
+     */
+    pageSize?: number;
+
+    /**
+     * Optionally an add button to display in the Modal. Can
+     * be used to dynamically add an option which was not there
+     * before.
+     */
+    addButton?: ModalPickerAddButtonOptions<T>;
+
+    /**
+     * Optionally the position the button should be aligned to
+     * within its container.
+     */
+    alignButton?: ModalPickerButtonAlignment;
+
+    /**
+     * Optionally callback to display the selected item.
+     */
+    renderValue?: ModalPickerSingleRenderValue<T>;
+
+    /**
+     * Callback to customize display of options.
+     */
+    renderOptions?: ModalPickerRenderOptions<T>;
+
+    /**
+     * Whether to show a "clear" button.
+     *
+     * Defaults to `true`
+     */
+    canClear?: boolean;
+
+    /**
+     * Optionally customized text within the component.
+     * This text should already be translated.
+     */
+    text?: Text;
+  };
 
 /**
  * The ModalPickerSingle is a form element which allows the user
@@ -130,11 +141,11 @@ export function ModalPickerSingle<T>(props: Props<T>) {
     text
   } = props;
 
-  const [ isOpen, setIsOpen ] = useState(false);
-  const [ pageNumber, setPageNumber ] = useState(1);
-  const [ query, setQuery ] = useState<string>('');
-  const [ userHasSearched, setUserHasSearched ] = useState(false);
-  const [ selected, setSelected ] = useState<T | undefined>(undefined);
+  const [isOpen, setIsOpen] = useState(false);
+  const [pageNumber, setPageNumber] = useState(1);
+  const [query, setQuery] = useState<string>('');
+  const [userHasSearched, setUserHasSearched] = useState(false);
+  const [selected, setSelected] = useState<T | undefined>(undefined);
 
   const { page, loading } = useOptions({
     options,
@@ -158,13 +169,14 @@ export function ModalPickerSingle<T>(props: Props<T>) {
     setIsOpen(false);
   }
 
-  function openModal() {
+  const openModal: MouseEventHandler<HTMLButtonElement> = (event) => {
+    event.preventDefault();
     setIsOpen(true);
     setSelected(value);
     setQuery('');
     setPageNumber(1);
     setUserHasSearched(false);
-  }
+  };
 
   async function addButtonClicked(callback: ModalPickerAddButtonCallback<T>) {
     setIsOpen(false);
@@ -189,11 +201,11 @@ export function ModalPickerSingle<T>(props: Props<T>) {
     renderValue: renderValue
       ? renderValue
       : (value: T) => (
-        <ModalPickerValueTruncator
-          value={value}
-          labelForOption={labelForOption}
-        />
-      ),
+          <ModalPickerValueTruncator
+            value={value}
+            labelForOption={labelForOption}
+          />
+        ),
     onClear: canClear ? () => onChange(undefined) : undefined,
     value
   };
@@ -219,9 +231,9 @@ export function ModalPickerSingle<T>(props: Props<T>) {
   function renderModal() {
     const addButtonOptions = addButton
       ? {
-        label: addButton.label,
-        onClick: () => addButtonClicked(addButton.onClick)
-      }
+          label: addButton.label,
+          onClick: () => addButtonClicked(addButton.onClick)
+        }
       : undefined;
 
     return (
@@ -243,13 +255,13 @@ export function ModalPickerSingle<T>(props: Props<T>) {
         renderOptionsConfig={
           renderOptions
             ? {
-              labelForOption,
-              isOptionEqual,
-              keyForOption,
-              isOptionEnabled,
-              renderOptions,
-              onChange: setSelected
-            }
+                labelForOption,
+                isOptionEqual,
+                keyForOption,
+                isOptionEnabled,
+                renderOptions,
+                onChange: setSelected
+              }
             : undefined
         }
         text={text}
@@ -292,9 +304,13 @@ export function ModalPickerSingle<T>(props: Props<T>) {
 /**
  * Variant of the ModalPickerSingle which can be used in a Jarb context.
  */
-export const JarbModalPickerSingle = withJarb<any, any | null, Props<any>>(ModalPickerSingle);
+export const JarbModalPickerSingle = withJarb<any, any | null, Props<any>>(
+  ModalPickerSingle
+);
 
 /**
  * Variant of the ModalPickerSingle which can be used in a final form.
  */
-export const FieldModalPickerSingle = withField<any, any | null, Props<any>>(ModalPickerSingle);
+export const FieldModalPickerSingle = withField<any, any | null, Props<any>>(
+  ModalPickerSingle
+);