Skip to content

Refactor overlay tool nodes and improve data table panning - #63

Merged
veillette merged 1 commit into
mainfrom
claude/review-class-design-O1PT6
Feb 27, 2026
Merged

Refactor overlay tool nodes and improve data table panning#63
veillette merged 1 commit into
mainfrom
claude/review-class-design-O1PT6

Conversation

@veillette

Copy link
Copy Markdown
Collaborator

Summary

This PR refactors shared behavior in overlay tool nodes into a reusable base class, moves data table panning logic into the DataTableNode itself, and makes several naming and implementation improvements across the codebase.

Key Changes

  • New DigitizingAwareOverlayNode base class: Extracted common visibility and interaction-locking behavior from CoordinateSystemNode and CalibrationToolNode into an abstract base class. Both nodes now:

    • Hide until a video is loaded (driven by videoLoadedProperty)
    • Become dimmed and non-interactive while the user is manually digitizing a track (driven by activeTrackIdProperty)
    • This eliminates ~30 lines of duplicated code
  • Data table panning refactored: Moved drag listener logic from SimScreenView into DataTableNode where it logically belongs, improving encapsulation and maintainability

  • Property naming improvement: Renamed isWebcamVideoProperty to isUserVideoProperty in VideoSourceModel for clarity—the property indicates whether the active video is user-provided (webcam or upload) vs. bundled sample video

  • Model-view transform handling: Simplified SimModel to use the lazyLink callback's oldMvt parameter instead of manually caching the previous transform

  • Minor type adjustments: Changed Node import to type Node in CalibrationToolNode and removed unused Transform3 import from SimModel

Implementation Details

The DigitizingAwareOverlayNode abstract base class handles property linking and cleanup in its constructor and dispose() method, allowing subclasses to focus on their specific UI logic. Both CoordinateSystemNode and CalibrationToolNode now call super(videoLoadedProperty, activeTrackIdProperty) to inherit this behavior automatically.

https://claude.ai/code/session_016hiUhJvruvQ8463TWCPYdf

- SimModel: remove prevModelViewTransform private field; lazyLink already
  provides the previous value as its second argument, so the manual cache
  was redundant and mutation-prone.

- DigitizingAwareOverlayNode: extract the shared "hide until video loaded +
  dim/lock during digitizing" pattern into an abstract base class.
  CoordinateSystemNode and CalibrationToolNode both extend it, removing
  ~25 lines of identical listener/dispose boilerplate from each.

- isWebcamVideoProperty → isUserVideoProperty: the old name was misleading
  because the flag is also true for uploaded files (not just webcam streams).
  The new name accurately reflects "user-provided vs bundled sample".

- DataTableNode: move the pan-drag listener inside the node itself.
  SimScreenView is a layout file and should not own interaction logic that
  belongs to the node being dragged.

https://claude.ai/code/session_016hiUhJvruvQ8463TWCPYdf
@veillette
veillette merged commit 23f4247 into main Feb 27, 2026
1 check passed
@veillette
veillette deleted the claude/review-class-design-O1PT6 branch March 1, 2026 01:13
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.

2 participants