refactor: increase usage of the MouseListenerSet interface#848
refactor: increase usage of the MouseListenerSet interface#848
Conversation
Changes: - This interface enforces the signature of “mouse” methods and makes it explicit that classes are listeners and can be added as such to Graph instances - It prevents “mouse” methods from being detected as unused by the IDE - Various enhancements have been made to JSDoc, in particular with regard to events launched by classes.
WalkthroughThis change primarily updates documentation comments, refines example code formatting, and clarifies parameter usage across several core files. It also explicitly implements the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Graph
participant MouseListenerSet
participant PanningHandler
participant PopupMenuHandler
participant SelectionHandler
User->>Graph: Triggers mouse event
Graph->>MouseListenerSet: Dispatches event to listeners
MouseListenerSet->>PanningHandler: Calls mouseDown/mouseMove/mouseUp(_sender, event)
MouseListenerSet->>PopupMenuHandler: Calls mouseDown/mouseMove/mouseUp(_sender, event)
MouseListenerSet->>SelectionHandler: Calls mouseDown/mouseMove/mouseUp(_sender, event)
Possibly related PRs
Suggested labels
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
packages/core/src/view/plugins/PanningHandler.ts (1)
234-236:⚠️ Potential issue
isActivecondition is wrong – handler is always “active”
initialScaleis numeric (default0). The current check againstnullis never false, soisActive()always returnstrue.- return this.active || this.initialScale !== null; + return this.active || this.initialScale !== 0;Correcting this restores the intended semantics (true only while panning or pinch-zooming).
packages/core/src/view/cell/CellTracker.ts (1)
90-96: 🛠️ Refactor suggestionAlign method signature with
MouseListenerSet
MouseListenerSet.mouseDownexpects(sender: EventSource, me: InternalMouseEvent).
Keeping the parameters (prefixed with_to mark them unused) avoids accidental breakage if stricter compiler flags (--strictFunctionTypes) are enabled later.- mouseDown() { - return; - } + mouseDown(_sender: EventSource, _me: InternalMouseEvent): void { + // intentionally left blank + }
🧹 Nitpick comments (2)
packages/core/src/view/plugins/PopupMenuHandler.ts (1)
47-50: Nit – prefix unused parameter ingestureHandlerfor lint consistency- this.gestureHandler = (sender: EventSource, eo: EventObject) => { + this.gestureHandler = (_sender: EventSource, eo: EventObject) => {All other handlers use the underscore pattern; aligning this one avoids a stray unused-var warning.
packages/core/src/gui/MaxPopupMenu.ts (1)
171-184: Nit: unify optional-parameter notation in JSDocThe new parameter list is clearer, but you could leverage JSDoc’s square-bracket notation for optional parameters to make it machine-parsable by more tooling:
- * @param image Optional URL for the image icon. + * @param [image] URL for the image icon.(Same for the remaining optional params.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/core/src/gui/MaxPopupMenu.ts(2 hunks)packages/core/src/view/GraphView.ts(1 hunks)packages/core/src/view/cell/CellMarker.ts(1 hunks)packages/core/src/view/cell/CellTracker.ts(2 hunks)packages/core/src/view/event/InternalMouseEvent.ts(1 hunks)packages/core/src/view/mixins/EventsMixin.type.ts(1 hunks)packages/core/src/view/other/PanningManager.ts(2 hunks)packages/core/src/view/plugins/PanningHandler.ts(8 hunks)packages/core/src/view/plugins/PopupMenuHandler.ts(5 hunks)packages/core/src/view/plugins/SelectionHandler.ts(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/core/src/view/plugins/PopupMenuHandler.ts (2)
packages/core/src/types.ts (2)
GraphPlugin(1178-1180)MouseListenerSet(1210-1214)packages/core/src/util/EventUtils.ts (1)
isMultiTouchEvent(93-102)
packages/core/src/view/plugins/SelectionHandler.ts (1)
packages/core/src/types.ts (2)
GraphPlugin(1178-1180)MouseListenerSet(1210-1214)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (windows-2022)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (macos-14)
- GitHub Check: build
🔇 Additional comments (8)
packages/core/src/view/event/InternalMouseEvent.ts (1)
35-44: Documentation update looks goodThe switch to concise arrow-function examples and consistent parameter naming improves readability without touching runtime behaviour.
packages/core/src/view/cell/CellMarker.ts (1)
41-58: Clearer JSDoc – nice polishThe refined example and “### Events” section bring the docs in line with the rest of the codebase. No functional impact detected.
packages/core/src/view/plugins/PopupMenuHandler.ts (1)
25-38: Interface declaration matches actual capabilitiesImplementing
MouseListenerSet(and importing it withtype) provides compile-time guarantees and silences IDE “unused” warnings. Implementation already fulfils the contract.packages/core/src/view/plugins/PanningHandler.ts (1)
34-70: Type-level conformance achievedAdding
MouseListenerSetkeeps IDEs honest;type-only import prevents runtime overhead.packages/core/src/view/plugins/SelectionHandler.ts (1)
527-545: Explicitly marking unused parameterRenaming
evtName→_evtNamecleanly suppresses the unused-parameter lint and mirrors other handlers.packages/core/src/view/mixins/EventsMixin.type.ts (2)
226-233: Correct and consistent doc-comment update – looks goodThe parameter reference was corrected to
InternalMouseEvent, and the surrounding prose became clearer. No issues spotted.
235-238: Back-ticked method names clarify the interface – nice touchUsing back-ticks around
mouseDown,mouseMove, andmouseUpprevents IDE mis-parsing and improves the generated docs.packages/core/src/gui/MaxPopupMenu.ts (1)
47-54: Event section re-formatting is an improvementExplicitly calling out
InternalEvent.SHOWin its own “Events” section makes the docs easier to scan.
|
ℹ️ The error detected by SonarCloud is not related to the changes provided in this PR. |
Changes:
Summary by CodeRabbit
Documentation
Refactor