Refactor Dialog scroll optimization to use DOM queries instead of ref counting

[Copilot]

The Dialog scroll optimization introduced in PR #7366 used module-level ref counting variables (optimizedScrollRefCount, legacyScrollRefCount) that persisted across tests and required manual cleanup. This refactors to use DOM queries as the source of truth.

Implementation

Removed:

• Module-level variables: optimizedScrollRefCount, legacyScrollRefCount
• Helper functions: enableOptimizedScroll(), disableOptimizedScroll(), enableLegacyScroll(), disableLegacyScroll()

Updated useEffect in _Dialog:

return () => {
  dialog?.classList.remove(classes.DisableScroll)
  
  // Query DOM to check if any other dialogs with DisableScroll remain
  const remainingDialogs = document.querySelectorAll(`.${classes.DisableScroll}`)
  
  if (remainingDialogs.length === 0) {
    // No more dialogs open, clean up body
    document.body.style.removeProperty('--prc-dialog-scrollgutter')
    document.body.classList.remove('DialogScrollDisabled')
    if (usePerfOptimization) {
      document.body.removeAttribute('data-dialog-scroll-optimized')
    }
  }
}

Body attributes/classes now persist until no dialogs with .DisableScroll remain in DOM. This also fixes an edge case where the original implementation could remove --prc-dialog-scrollgutter prematurely when multiple dialogs were open.

Tests:

• Removed beforeEach cleanup hook (DOM resets naturally between tests)
• All existing test coverage maintained

Changelog

Changed

• Dialog scroll optimization now uses DOM queries instead of module-level ref counting
• Body scroll attributes/classes cleanup is deferred until all dialogs are closed

Removed

• Module-level ref counting variables and helper functions

Rollout strategy

• Patch release
• Minor release
• Major release; if selected, include a written rollout or migration plan
• None; if selected, include a brief description as to why

Testing & Reviewing

All 27 Dialog tests pass, including multi-dialog scenarios and feature flag variations. Full test suite (2,211 tests) passes.

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Storybook)
• Changes are SSR compatible
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge
• (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

Original prompt

Summary

Refactor the Dialog scroll optimization in PR #7366 to eliminate module-level ref counting variables (optimizedScrollRefCount, legacyScrollRefCount) and instead use DOM queries to determine when to clean up body attributes/classes.

Problem

The current implementation uses module-level variables for ref counting:

• optimizedScrollRefCount
• legacyScrollRefCount

This creates several issues:

1. Global state that persists across tests, causing potential test pollution
2. State that needs manual reset in test cleanup
3. Unnecessary complexity when the DOM already tracks which dialogs are open

Solution

Use Option 1: Pure CSS + DOM query approach:

1. Remove the module-level ref counting variables and their helper functions (enableOptimizedScroll, disableOptimizedScroll, enableLegacyScroll, disableLegacyScroll)

2. Update the useEffect in _Dialog to:

   • On mount: Add the DisableScroll class to the dialog, set the scrollbar gutter CSS variable, and if the feature flag is ON, set data-dialog-scroll-optimized attribute and DialogScrollDisabled class on body
   • On cleanup: Remove the DisableScroll class from the dialog, then query the DOM to check if any other dialogs with .DisableScroll class remain. Only remove body attributes/classes if no other dialogs are open.

3. Update the CSS to work with both paths:

   • Legacy path: body:not([data-dialog-scroll-optimized]):has(.Dialog.DisableScroll)
   • Optimized path: body.DialogScrollDisabled (only applied when flag is ON)

Implementation in Dialog.tsx

React.useEffect(() => {
  const scrollbarWidth = window.innerWidth - document.body.clientWidth
  const dialog = dialogRef.current
  
  // Add DisableScroll class to this dialog
  dialog?.classList.add(classes.DisableScroll)
  document.body.style.setProperty('--prc-dialog-scrollgutter', `${scrollbarWidth}px`)

  if (usePerfOptimization) {
    // Optimized path: set attribute and class on body
    document.body.setAttribute('data-dialog-scroll-optimized', '')
    document.body.classList.add('DialogScrollDisabled')
  }

  return () => {
    // Remove DisableScroll class from this dialog
    dialog?.classList.remove(classes.DisableScroll)
    
    // Query DOM to check if any other dialogs with DisableScroll remain
    const remainingDialogs = document.querySelectorAll(`.${classes.DisableScroll}`)
    
    if (remainingDialogs.length === 0) {
      // No more dialogs open, clean up body
      document.body.style.removeProperty('--prc-dialog-scrollgutter')
      if (usePerfOptimization) {
        document.body.removeAttribute('data-dialog-scroll-optimized')
        document.body.classList.remove('DialogScrollDisabled')
      }
    }
  }
}, [usePerfOptimization])

4. Update tests to remove any ref count reset logic since it's no longer needed. The tests should still verify:
   • Flag OFF: no data-dialog-scroll-optimized attribute, but body scroll is still disabled via CSS :has()
   • Flag ON: data-dialog-scroll-optimized attribute and DialogScrollDisabled class present
   • Multiple dialogs: attributes/classes persist until ALL dialogs are closed

Files to modify

1. packages/react/src/Dialog/Dialog.tsx - Remove ref counting, implement DOM query approach
2. packages/react/src/Dialog/Dialog.test.tsx - Update tests to remove ref count reset if present, verify DOM query behavior works correctly

Benefits

• No global/module-level state
• DOM is the source of truth
• Tests naturally reset because DOM resets between tests
• Works correctly with mixed Turbo/React dialogs
• Simpler implementation

This pull request was created from Copilot chat.

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[changeset-bot]

⚠️ No Changeset found

Latest commit: c00c860

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR