Tidy up shared React components (#9509)

GitOrigin-RevId: b8029b65c95ccdac89587d484e2599e25fe44302

services/web/frontend/js/shared/components/accessible-modal.tsx

@@ -1,31 +1,30 @@
 import { useCallback } from 'react'
 import { Modal, ModalProps } from 'react-bootstrap'
 
-// a bootstrap Modal with its `aria-hidden` attribute removed. Visisble modals
-// should not have their `aria-hidden` attribute set but that's a bug in our
-// version of react-bootstrap.
-function AccessibleModal({ show, ...otherProps }: ModalProps) {
-  // use a callback ref to track the modal. This will re-run the function
-  // when the element node or any of the dependencies are updated
-  const setModalRef = useCallback(
+// A wrapper for the v0.33 React Bootstrap Modal component,
+// which ensures that the `aria-hidden` attribute is not set on the modal when it's visible,
+// and that role="dialog" is not duplicated.
+// https://github.com/react-bootstrap/react-bootstrap/issues/4790
+// There are other ARIA attributes on these modals which could be improved,
+// but this at least makes them accessible for tests.
+function AccessibleModal(props: ModalProps) {
+  const modalRef = useCallback(
     element => {
-      if (!element) return
-
-      const modalNode = element._modal && element._modal.modalNode
-      if (!modalNode) return
-
-      if (show) {
-        modalNode.removeAttribute('aria-hidden')
-      } else {
-        modalNode.setAttribute('aria-hidden', 'true')
+      const modalNode = element?._modal?.modalNode
+      if (modalNode) {
+        if (props.show) {
+          modalNode.removeAttribute('role')
+          modalNode.removeAttribute('aria-hidden')
+        } else {
+          // NOTE: possibly not ever used, as the modal is only rendered when shown
+          modalNode.setAttribute('aria-hidden', 'true')
+        }
       }
     },
-    // `show` is necessary as a dependency, but eslint thinks it is not
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [show]
+    [props.show]
   )
 
-  return <Modal show={show} {...otherProps} ref={setModalRef} />
+  return <Modal {...props} ref={modalRef} />
 }
 
 export default AccessibleModal

services/web/frontend/js/shared/components/beta-badge.tsx

@@ -1,24 +1,20 @@
+import type { FC, ReactNode } from 'react'
+import classnames from 'classnames'
 import Tooltip from './tooltip'
 import { OverlayTriggerProps } from 'react-bootstrap'
 
 type TooltipProps = {
   id: string
-  text: React.ReactNode
+  text: ReactNode
   placement?: OverlayTriggerProps['placement']
   className?: string
 }
 
-type BetaBadgeProps = {
+const BetaBadge: FC<{
   tooltip: TooltipProps
   url?: string
   phase?: string
-}
-
-function BetaBadge({
-  tooltip,
-  url = '/beta/participate',
-  phase = 'beta',
-}: BetaBadgeProps) {
+}> = ({ tooltip, url = '/beta/participate', phase = 'beta' }) => {
   let badgeClass
   switch (phase) {
     case 'release':
@@ -46,7 +42,7 @@ function BetaBadge({
         href={url}
         target="_blank"
         rel="noopener noreferrer"
-        className={`badge ${badgeClass}`}
+        className={classnames('badge', badgeClass)}
       >
         <span className="sr-only">{tooltip.text}</span>
       </a>

services/web/frontend/js/shared/components/controlled-dropdown.js

@@ -1,37 +0,0 @@
-import React from 'react'
-import { Dropdown } from 'react-bootstrap'
-import PropTypes from 'prop-types'
-import useDropdown from '../hooks/use-dropdown'
-
-export default function ControlledDropdown(props) {
-  const dropdownProps = useDropdown(Boolean(props.defaultOpen))
-
-  return (
-    <Dropdown {...props} {...dropdownProps}>
-      {React.Children.map(props.children, child => {
-        if (!React.isValidElement(child)) {
-          return child
-        }
-
-        // Dropdown.Menu
-        if ('open' in child.props) {
-          return React.cloneElement(child, { open: dropdownProps.open })
-        }
-
-        // Overlay
-        if ('show' in child.props) {
-          return React.cloneElement(child, { show: dropdownProps.open })
-        }
-
-        // anything else
-        return React.cloneElement(child)
-      })}
-    </Dropdown>
-  )
-}
-ControlledDropdown.propTypes = {
-  children: PropTypes.any,
-  defaultOpen: PropTypes.bool,
-  id: PropTypes.string,
-  className: PropTypes.string,
-}

services/web/frontend/js/shared/components/controlled-dropdown.tsx

@@ -0,0 +1,34 @@
+import { Children, cloneElement, type FC, isValidElement } from 'react'
+import { Dropdown, DropdownProps } from 'react-bootstrap'
+import useDropdown from '../hooks/use-dropdown'
+
+const ControlledDropdown: FC<
+  DropdownProps & { defaultOpen?: boolean }
+> = props => {
+  const dropdownProps = useDropdown(Boolean(props.defaultOpen))
+
+  return (
+    <Dropdown {...props} {...dropdownProps}>
+      {Children.map(props.children, child => {
+        if (!isValidElement(child)) {
+          return child
+        }
+
+        // Dropdown.Menu
+        if ('open' in child.props) {
+          return cloneElement(child, { open: dropdownProps.open })
+        }
+
+        // Overlay
+        if ('show' in child.props) {
+          return cloneElement(child, { show: dropdownProps.open })
+        }
+
+        // anything else
+        return cloneElement(child)
+      })}
+    </Dropdown>
+  )
+}
+
+export default ControlledDropdown

services/web/frontend/js/shared/components/tooltip.tsx

@@ -1,28 +1,20 @@
+import type { FC, ReactNode } from 'react'
 import {
   OverlayTrigger,
   OverlayTriggerProps,
   Tooltip as BSTooltip,
 } from 'react-bootstrap'
 
-type OverlayTriggerCustomProps = {
+type OverlayProps = Omit<OverlayTriggerProps, 'overlay'> & {
   shouldUpdatePosition?: boolean // Not officially documented https://stackoverflow.com/a/43138470
-} & OverlayTriggerProps
+}
 
-type TooltipProps = {
-  children: React.ReactNode
-  description: React.ReactNode
+const Tooltip: FC<{
+  description: ReactNode
   id: string
-  overlayProps?: Omit<OverlayTriggerCustomProps, 'overlay'>
+  overlayProps?: OverlayProps
   tooltipProps?: BSTooltip.TooltipProps
-}
-
-function Tooltip({
-  id,
-  description,
-  children,
-  tooltipProps,
-  overlayProps,
-}: TooltipProps) {
+}> = ({ id, description, children, tooltipProps, overlayProps }) => {
   return (
     <OverlayTrigger
       overlay={

services/web/test/frontend/components/shared/accessible-modal.spec.tsx

@@ -0,0 +1,36 @@
+import { Modal } from 'react-bootstrap'
+import AccessibleModal from '../../../../frontend/js/shared/components/accessible-modal'
+
+describe('AccessibleModal', function () {
+  it('renders a visible modal', function () {
+    const handleHide = cy.stub()
+
+    cy.mount(
+      <AccessibleModal onHide={handleHide} show>
+        <Modal.Header closeButton>
+          <Modal.Title>Test</Modal.Title>
+        </Modal.Header>
+
+        <Modal.Body>Some content</Modal.Body>
+      </AccessibleModal>
+    )
+
+    cy.findByRole('dialog').should('have.length', 1)
+  })
+
+  it('does not render a hidden modal', function () {
+    const handleHide = cy.stub()
+
+    cy.mount(
+      <AccessibleModal onHide={handleHide}>
+        <Modal.Header closeButton>
+          <Modal.Title>Test</Modal.Title>
+        </Modal.Header>
+
+        <Modal.Body>Some content</Modal.Body>
+      </AccessibleModal>
+    )
+
+    cy.findByRole('dialog', { hidden: true }).should('have.length', 0)
+  })
+})