Skip to content

Conversation

DanielSinclair
Copy link
Collaborator

@DanielSinclair DanielSinclair commented Sep 10, 2025

Summary

  • avoid triggering home hotkeys when dapp network switcher is active
  • Refactored active element detection - renamed getInputIsFocused() → inputIsFocused(), added modalIsActive()
  • Restored switchNetworkMenuIsActive() - prevents wallet switcher shortcuts when network menu is open
  • Removed isExplainerSheet prop - replaced with generic modalIsActive() check
  • Updated useKeyboardShortcut hook - evaluates conditions dynamically, includes all deps in useEffect
  • Improved overlay blocking - CommandK, home shortcuts, and wallet switcher all respect modal/menu state

https://chatgpt.com/codex/tasks/task_e_68c053a8fb088325beb125629a56a32b


PR-Codex overview

This PR focuses on improving the handling of keyboard shortcuts and modal interactions across various components in the application. It introduces new functions for detecting active modals and input focus, while also refining existing logic for better usability.

Detailed summary

  • Changed isExplainerSheet prop to be removed from Box in BackupReminder.
  • Updated getInputIsFocused to inputIsFocused in multiple files.
  • Introduced modalIsActive function to check for active modals.
  • Refined keyboard shortcut conditions to consider modal and input focus states.
  • Improved overall clarity and consistency in shortcut handling across components.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@DanielSinclair DanielSinclair marked this pull request as draft September 11, 2025 04:10
Copy link

linear bot commented Sep 11, 2025

@DanielSinclair DanielSinclair force-pushed the codex/fix-dapp-network-switcher-hotkeys branch from 12274f9 to 95701a2 Compare October 14, 2025 20:40
fix: block switch wallet when modal is visible

fix: disable shortcuts during backup reminder

fix: avoid intercepting clicks after testnet toggle

fix: simplify preventDefault calls

refactor: consolidate and simplify active element detection

- Rename functions to follow consistent *IsActive pattern:
  - getInputIsFocused → inputIsFocused
  - getActiveModal → modalIsActive
  - getExplainerSheet → explainerSheetIsActive

- Remove redundant detection functions:
  - switchNetworkMenuIsActive (now use modalIsActive + radixIsActive)
  - backupReminderIsActive (already caught by modalIsActive via BottomSheet)
  - explainerSheetIsActive (redundant with modalIsActive)

- Add clear documentation and organize into logical sections:
  - General Element Helpers
  - Modal and Sheet Detectors
  - Menu Detectors

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

refactor: remove redundant isExplainerSheet prop

ExplainerSheet and BackupReminder components are already wrapped
in BottomSheet which sets isModal, so they're automatically caught
by modalIsActive(). The isExplainerSheet prop was redundant.

- Remove isExplainerSheet prop from Box component
- Remove isExplainerSheet usage from ExplainerSheet
- Remove isExplainerSheet usage from BackupReminder

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

refactor: use radixIsActive for dropdown menu detection

DropdownMenu components use Radix UI primitives which are already
detected by radixIsActive() via data-radix-popper-content-wrapper.
No need for isModal prop.

- Remove isModal from DropdownMenuContent
- Remove element ID from SwitchNetworkMenu (no longer needed)

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

refactor: update callers to use simplified activeElement functions

- Rename getInputIsFocused → inputIsFocused across all files
- Rename getActiveModal → modalIsActive in Navbar
- Replace switchNetworkMenuIsActive with modalIsActive + radixIsActive
- Replace backupReminderIsActive with modalIsActive
- Remove getExplainerSheet usage from hooks
- Restore clickTabBar call in testnet mode toggle

fix: check DOM state in handler, not condition callback

- Remove modalIsActive() check from getHomeShortcutsAreActive
- modalIsActive() and radixIsActive() are now only checked in handleHomeShortcuts
- Condition callback focuses on React state (sheet, transaction, token)
- Handler performs runtime DOM state checks (input focus, modals, radix)
- Prevents stale DOM state from being cached in condition dependencies

refactor: improve type safety for modal detection functions

Restore getActiveModal() to return the actual modal element for cases like simulateTab that need to query within the modal. Make modalIsActive() explicitly return boolean for consistency with other detection functions.

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

fix: evaluate condition on every keypress in useKeyboardShortcut

Previously, the condition callback was only evaluated during the useMemo calculation of shouldListen, not on every keypress. This meant that when modals like BackupReminder opened after the hook was registered, shortcuts would still fire because the condition check was stale.

Now the condition is evaluated inside the keyboard event handler on every keypress, ensuring real-time detection of modal state changes.

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

fix: keep command palette shortcuts active

fix: unnecessary conditions

revert: radixIsActive changes
@DanielSinclair DanielSinclair force-pushed the codex/fix-dapp-network-switcher-hotkeys branch from 9c1b626 to 16ee8f9 Compare October 15, 2025 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant