Skip to content

fix(ui-modal): keep content visible in safari after nested transitions - #2735

Open
joyenjoyer wants to merge 1 commit into
masterfrom
INSTUI-5200-togglegroup-safari-invisible
Open

joyenjoyer wants to merge 1 commit into
masterfrom
INSTUI-5200-togglegroup-safari-invisible

Conversation

@joyenjoyer

Copy link
Copy Markdown
Contributor

Summary

  • Add isolation: isolate to the v1 and v2 ModalBody styles. Safari sometimes doesn't repaint text in a tall, scrolling Modal body after a fade-in transition ends (e.g. nested ToggleGroups), so the text stays invisible until selected.

Test Plan

  • In Safari, paste the snippet below into a docs site example, open the Modal, and expand both groups. The row text should stay visible.
  • Spot-check a few Modals with Popover/Select content in Chrome, Firefox, and Safari for layering changes.
Repro snippet (docs site example)
const labels = Array.from({ length: 80 }, (_, i) => `Homework ${i + 1}`)

const Row = ({ label }) => (
  <Flex>
    <Flex.Item margin="0 small 0 0">
      <Checkbox label={<ScreenReaderContent>{label}</ScreenReaderContent>} />
    </Flex.Item>
    <Flex.Item shouldShrink>
      <Text>{label}</Text>
    </Flex.Item>
  </Flex>
)

const Group = ({ label, children }) => (
  <ToggleGroup
    toggleLabel={`${label}, View All`}
    border={false}
    summary={<Text aria-hidden="true">{label}</Text>}
  >
    {children}
  </ToggleGroup>
)

const Example = () => {
  const [open, setOpen] = useState(false)
  return (
    <View as="div">
      <Button onClick={() => setOpen(true)}>Select content</Button>
      <Modal
        open={open}
        onDismiss={() => setOpen(false)}
        size="medium"
        label="Select Content for Import"
        shouldCloseOnDocumentClick
      >
        <Modal.Header>
          <Heading>Select Content for Import</Heading>
        </Modal.Header>
        <Modal.Body>
          <Group label="Assignments">
            <Group label="Assignment Group 1">
              {labels.map((l) => (
                <Row key={l} label={l} />
              ))}
            </Group>
          </Group>
        </Modal.Body>
        <Modal.Footer>
          <Button onClick={() => setOpen(false)}>Cancel</Button>
        </Modal.Footer>
      </Modal>
    </View>
  )
}

render(<Example />)

Fixes INSTUI-5200

🤖 Generated with Claude Code

Safari sometimes doesn't repaint text in a tall, scrolling Modal body after a fade-in transition
ends, e.g. with nested ToggleGroups, so the text stays invisible until selected. Make the Modal
body a stacking context with `isolation: isolate` in v1 and v2, which avoids it.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@joyenjoyer joyenjoyer self-assigned this Sep 30, 2026
@joyenjoyer
joyenjoyer requested a review from HerrTopi September 30, 2026 11:54
@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-2735/

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

github-actions Bot pushed a commit that referenced this pull request Sep 30, 2026
@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.

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