Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui-a11y-utils): handleDocumentClick should only capture mouse events #1332

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

balzss
Copy link
Contributor

@balzss balzss commented Oct 26, 2023

INSTUI-3907

explanation: for shouldCloseOnDocumentClick we used to have two mousedown listeners: one for capturing the document click (store the target) and one for handling it (close document if target is outside the context, e.g. modal). the latter were converted to a click listener to fix the order of execution but that introduced this bug because the click listener can also capture events like pressing space on a checkbox (for a11y reasons). the solution is to use a mouseup listener instead of a click listener which isn't triggered by the spacebar

https://stackoverflow.com/questions/27878940/spacebar-triggering-click-event-on-checkbox

test plan

  • open the docs app at the modal example
  • use this code:
<div style={{ padding: '0 0 11rem 0', margin: '0 auto' }}>
        <Button onClick={this.handleButtonClick}>
          {this.state.open ? 'Close' : 'Open'} the Modal
        </Button>
        <Modal
          as="form"
          open={this.state.open}
          onDismiss={() => { this.setState({ open: false }) }}
          onSubmit={this.handleFormSubmit}
          size="auto"
          label="Modal Dialog: Hello World"
          shouldCloseOnDocumentClick
        >
          <Modal.Header>
            {this.renderCloseButton()}
            <Heading>Hello World</Heading>
          </Modal.Header>
          <Modal.Body>
            <TextInput renderLabel="Example" placeholder="if you hit enter here, it should submit the form" />
            <Text lineHeight="double">{fpo}</Text>
            <Checkbox label={lorem.sentence()} value="medium" defaultChecked />
          </Modal.Body>
          <Modal.Footer>
            <Button onClick={this.handleButtonClick} margin="0 x-small 0 0">Close</Button>
            <Button color="primary" type="submit">Submit</Button>
          </Modal.Footer>
        </Modal>
      </div>
  • open the modal
  • press tabs until the focus is on the checkbox
  • press space
  • the modal shouldn't close

@balzss balzss requested review from matyasf and joyenjoyer October 26, 2023 10:37
@balzss balzss self-assigned this Oct 26, 2023
@balzss balzss force-pushed the fix/checkbox-closes-modal branch from e94f168 to 53bed32 Compare October 27, 2023 10:59
@balzss balzss force-pushed the fix/checkbox-closes-modal branch from 53bed32 to ebc91f4 Compare October 27, 2023 11:45
@balzss balzss force-pushed the fix/checkbox-closes-modal branch from 8215353 to 61e684b Compare October 27, 2023 12:49
@github-actions
Copy link

Preview URL: https://1332--preview-instui.netlify.app

@balzss balzss merged commit abcd31f into master Oct 27, 2023
4 checks passed
@balzss balzss deleted the fix/checkbox-closes-modal branch October 27, 2023 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants