grey out modals when focus is outside - Eagle 1558 - #1076
Merged
Merged
Conversation
Contributor
Reviewer's GuideThe PR introduces capture-phase, self-cleaning modal mousedown tracking to visually grey out draggable modal headers when interaction moves outside modal content, while preserving modal lifecycle and drag behavior. It also clarifies the graph deselection API and shortcut naming and removes an unnecessary console log. Sequence diagram for modal focus-away trackingsequenceDiagram
participant User
participant Document
participant Modal as Open modal
participant ModalContent as Modal content
User->>Document: mousedown
Document->>Modal: modalFocusStateHandler(event)
alt target inside .modal-content
Modal->>Modal: removeClass(modal-focus-away)
else target outside .modal-content
Modal->>Modal: addClass(modal-focus-away)
end
User->>ModalContent: mousedown on modal header
ModalContent->>Modal: removeClass(modal-focus-away)
State diagram for modal focus listener lifecyclestateDiagram-v2
[*] --> NoModal
NoModal --> ModalOpen: show.bs.modal / attachModalFocusListener
ModalOpen --> ModalOpen: mousedown / update modal-focus-away
ModalOpen --> NoModal: hide.bs.modal and no .modal.show / detachModalFocusListener
ModalOpen --> ModalOpen: hide.bs.modal while another modal is shown
Flow diagram for graph deselection shortcut renameflowchart LR
Escape[Escape shortcut] --> Shortcut[select_no_objects_in_graph]
Shortcut --> Method[Eagle.selectNoObjectsInGraph]
Method --> Clear["selectedObjects([])"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/Eagle.ts" line_range="5787-5788" />
<code_context>
$('.modal .modal-content').css({"pointerEvents":"auto"})
+
+ // Keep the listener alive if another modal is taking over during this transition.
+ if ($('.modal.show').length === 0) {
+ detachModalFocusListener();
+ }
});
</code_context>
<issue_to_address>
**issue (bug_risk):** The listener is not detached when the last modal is closed because this check runs in the `hide.bs.modal` handler while the closing modal still has the `.show` class, so `$('.modal.show').length` is nonzero. The document-level `mousedown` listener therefore remains attached permanently after the first modal is used.
**Triggers:** After a modal has been opened and then closed.
**Suggested fix:** Move the check to a `hidden.bs.modal` handler, or defer the check until the modal has finished hiding.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: src/Eagle.ts:5788
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
added an event listener that cleans itself up when no modals are being used
if the modal is in a draggable state, it's header will appear greyed out when the user is interacting with ui outside of the modal
renamed a function to be more indicative of what it does, and removed an unnecessary console.log
Summary by Sourcery
Add focus-away feedback for draggable modals and improve modal listener lifecycle management.
New Features:
Bug Fixes:
Enhancements: