Fix Modal focus jumping from child to itself

[r100-stack]

Changes

A user reported that since 3.17.0, when typing in a controlled input inside a Modal, the focus jumps from the input to the role="dialog" whenever a character is typed. On investigating it seems to be a regression in 3.17.0 because of #2391.

The issue was that onEnter was being called each time the ref function was being called. Thus, onEnter was calling .focus() on the dialog multiple times.

Looks like the ref function was being called multiple times because of the <FocusTrap>. Since focus trap is enabled by default in Modal but not in Dialog, this issue was there in the Modal but not in the Dialog.

The exact issue is the cloneElementWithRef part:

iTwinUI/packages/itwinui-react/src/utils/components/FocusTrap.tsx

Line 52 in f744dc5

{cloneElementWithRef(children, () => ({ ref: childRef }))}

Memoizing children too didn't work. So I removed the cloning by adding the ref to the first focus trap div and then accessing the children element with firstFocusTrapRef.current?.nextElementSibling. (4dde95f).

Testing

Confimed in a playground similar to the user's to confirm that the focus jump does not happen in Dialog and Modal.

Code

import * as React from 'react';
import {
  Button,
  Dialog,
  InputGroup,
  LabeledInput,
  Modal,
  ModalButtonBar,
  ModalContent,
} from '@itwin/itwinui-react';

export default function App() {
  const [isModalOpen, setIsModalOpen] = React.useState(false);
  const [isDialogOpen, setIsDialogOpen] = React.useState(false);
  const [value, setValue] = React.useState('');

  return (
    <>
      <Button styleType='high-visibility' onClick={() => setIsModalOpen(true)}>
        Open Modal
      </Button>
      <Button styleType='high-visibility' onClick={() => setIsDialogOpen(true)}>
        Open Dialog
      </Button>

      <Modal
        isOpen={isModalOpen}
        title={'Modal title'}
        onClose={() => setIsModalOpen(false)}
        closeOnEsc
        closeOnExternalClick
      >
        <ModalContent>
          <InputGroup>
            <LabeledInput
              label='Input label'
              value={value}
              onChange={(event) => setValue(event.target.value)}
            />
          </InputGroup>
        </ModalContent>
        <ModalButtonBar>
          <Button styleType='high-visibility'>Create</Button>
          <Button>Cancel</Button>
        </ModalButtonBar>
      </Modal>

      <Dialog
        isOpen={isDialogOpen}
        onClose={() => setIsDialogOpen(false)}
        closeOnEsc
        isDismissible
      >
        <Dialog.Main>
          <Dialog.TitleBar titleText='Best dialog ever' />
          <Dialog.Content>
            <InputGroup>
              <LabeledInput
                label='Input label'
                value={value}
                onChange={(event) => setValue(event.target.value)}
              />
            </InputGroup>
          </Dialog.Content>
          <Dialog.ButtonBar>
            <Button styleType='high-visibility'>Primary</Button>
            <Button>Secondary</Button>
          </Dialog.ButtonBar>
        </Dialog.Main>
      </Dialog>
    </>
  );
}

Also added small e2e tests for Modal and Dialog to confirm this.

Docs

Added a changeset.

After PR TODOs

• Investigate if cloneElementWithRef causes re-renders and any problems associated with it (#2443 (comment))