Skip to content

fix(ui-dialog,ui-modal): fix flaky Modal and DrawerLayout tests - #2734

Open
matyasf wants to merge 1 commit into
masterfrom
fix/INSTUI-5201-flaky-tests
Open

matyasf wants to merge 1 commit into
masterfrom
fix/INSTUI-5201-flaky-tests

Conversation

@matyasf

@matyasf matyasf commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog reads its props when the focus region activates (one frame after open()), so props that change in between aren't lost. This fixes the flaky DrawerLayout Escape test and the mount-in-overlay case of INSTUI-4401 (not its resize case).
  • New Dialog regression test for props that change before the focus region activates.
  • Modal Tooltip tests now wait for the tooltip to hide instead of checking once. fixing random test failures

Test Plan

  • In the docs app, load a DrawerLayout example narrow enough to start in overlay mode, open the tray, and check that Escape closes it and Tab stays inside it.

check this test code:

<div>
  <Flex gap="large" alignItems="start">
    <div>
      <RadioInputGroup name="fruit1">
        <RadioInput label="Apple" value="apple" />
      </RadioInputGroup>
    </div>
    <div>
      <RadioInputGroup
      name="fruit2"
      messages={[
        { type: 'screenreader-only', text: 'Pick one option' }
      ]}
      >
        <RadioInput label="SR only message" value="apple" />
      </RadioInputGroup>
    </div>
  </Flex>
  <RadioInputGroup
    messages={[{type:'success', text: "asd asdasd asdasd asdas"}, {type:'hint', text: "asd asdasd asdasd asdas"}]}
    name="example2"
    description="inline RadioInputGroup"
    layout="inline">
    <RadioInput value='sdfsdf' label='sdfsdf' />
  </RadioInputGroup>
  <RadioInputGroup
    messages={[{type:'error', text: "asd asdasd asdasd asdas"}, {type:'hint', text: "asd asdasd asdasd asdas"}]}
    name="example2"
    description="stacked RadioInputGroup"
    layout="stacked">
    <RadioInput value='sdfsdf' label='sdfsdf' />
  </RadioInputGroup>
    <FormFieldGroup
    messages={[{type:'error', text: "asd asdasd asdasd asdas"}, {type:'hint', text: "asd asdasd asdasd asdas"}]}
    description="Breakfast"
    rowSpacing="small"
    layout="inline"
    vAlign="middle"
  >
    <TextInput renderLabel="Favorite Breakfast Eatery"/>
    <RadioInputGroup
      name="beverage"
      description="Beverage of Choice"
      defaultValue="coffee"
      layout="columns"
    >
      <RadioInput label="Juice" value="juice" />
      <RadioInput label="Water" value="water" />
    </RadioInputGroup>
  </FormFieldGroup>
  <CheckboxGroup
  name="sports2"
  layout="inline"
  messages={[
    { text: 'Invalid name', type: 'error' }
  ]}
  onChange={function (value) { console.log(value) }}
  defaultValue={['soccer', 'volleyball']}
  description="I wish to receive score alerts for"
>
  <Checkbox label="Football" value="football" variant="toggle" />
  <Checkbox label="Basketball" value="basketball" variant="toggle" />
</CheckboxGroup>
    <CheckboxGroup
  name="sports2"
  layout="stacked"
  messages={[
    { text: 'Invalid name', type: 'error' }
  ]}
  onChange={function (value) { console.log(value) }}
  defaultValue={['soccer', 'volleyball']}
  description="I wish to receive score alerts for"
>
  <Checkbox label="Football" value="football" variant="toggle" />
  <Checkbox label="Basketball" value="basketball" variant="toggle" />
</CheckboxGroup>
</div>

Fixes INSTUI-5201

🤖 Generated with Claude Code

Dialog read its props in open() but built its focus region one animation frame later, so
props that changed in between were lost. DrawerTray turns on shouldCloseOnEscape only after
the layout switches to overlay mode, so the region sometimes ignored Escape. Dialog now reads
its props inside the frame callback, and a new regression test covers it. This also fixes
the mount-in-overlay case of INSTUI-4401, but not its resize case.

The Modal Tooltip tests checked once that the tooltip was hidden right after the modal
opened. Position hides offscreen content only after a delayed update, so the tests now
wait for it.

Refs: INSTUI-5201

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@matyasf matyasf self-assigned this Sep 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://instructure.design/pr-preview/pr-2734/

Built to branch gh-pages at 2026-09-30 08:18 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@matyasf
matyasf requested a review from git-nandor September 30, 2026 08:19
@github-actions

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff: ⚠️ Changes detected.

Status Count
Unchanged 98
Changed 1
New 0
Removed 0

Accessibility (axe): ✅ No violations.

📊 View full report — click a screenshot's ⚠ badge to see each violation boxed on the image, with the offending element named and contrast failures shown as color swatches.

Diff images (1)

badge-canvas.png — 1573 pixels differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

github-actions Bot pushed a commit that referenced this pull request Sep 30, 2026

This branch has not been deployed

No deployments
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.

1 participant