Modernize Navigation and Tool UX - #239
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #239 +/- ##
==========================================
+ Coverage 25.08% 25.31% +0.22%
==========================================
Files 528 529 +1
Lines 44211 44314 +103
Branches 4793 4795 +2
==========================================
+ Hits 11092 11218 +126
+ Misses 33119 33096 -23 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
79d10cf to
1cb59c3
Compare
- Remove legacy automatic edge-panning and continuous scroll timer from all mouse modes. - Implement a universal panning layer: Middle Mouse Button drag, Spacebar + Left Mouse Button drag, native trackpad/mouse smooth scrolling (pixelDelta), and two-finger touch panning, all usable concurrently with tool interactions. - Transition Connection, Area Selection, and Infomark tools from drag-based to click-to-click workflows. - Revamp Connection tool UX: cyan ghost lines and green snap points that work regardless of panning, depth-tested on top, right-click cancellation, and chaining that starts the next connection from the target room without a pre-selected direction. - Centralize and unit-test room-relative direction snapping logic in src/map/ExitDirection.cpp. - Add a 3-pixel movement threshold before showing tooltips in MOVE mode, and synchronize the tool cursor during panning to prevent visual stutter. - Upgrade the project to the C++20 standard across all CMake configurations and fix resulting migration issues (concepts.h, public Crtp constructors, std::unordered_map transparency in emojis.cpp). - Add TestUIState unit tests covering the modernized input state machine.
SELECT_CONNECTIONS and CREATE_CONNECTIONS previously cleared the whole in-progress selection whenever the second click didn't land on a valid matching exit, forcing the user to restart from scratch. Re-anchor on that click instead, mirroring the chaining already used on success in CREATE_CONNECTIONS, so a near-miss just re-aims rather than losing progress. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
Re-anchoring on a miss (previous commit) turned out to grab a new anchor on almost every click, since most clicks land near some exit in that room — it looked like clicking always "selected" whichever room was clicked. Instead, an invalid second click now just clears the tentative second point and leaves the original anchor in place; right-click remains the explicit way to cancel. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
mouseMoveEvent only called selectionChanged() for SELECT_CONNECTIONS and CREATE_CONNECTIONS when a connection anchor already existed, so the nearby-exit hover dots (cyan/green points in paintNearbyConnectionPoints) never repainted while just moving the mouse before the first click, making them look stale or misplaced rather than tracking the cursor live. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
paintSelectedRooms() was highlighting the connection-building anchor room using the exact same texture/quad as an actual room selection (room-sel.png), making it visually indistinguishable from selecting the room outright. The anchor already has its own distinct green point + ghost line drawn in paintSelectedConnection(), so drop the redundant room-selection-style highlight. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
Cyan now consistently means "not yet selected" (nearby exit candidates, pending ghost line), and green is reserved for two things: the anchor point itself (so it stands out among the cyan candidates) and the ghost line once the current anchor+target form a complete, valid connection. Previously every candidate target was drawn green as soon as an anchor existed, which gave green no clear meaning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
After a valid SELECT_CONNECTIONS second click completed a selection, the active ConnectionInteraction ended, so the next click (even an unrelated stray one that doesn't land on a valid exit) fell into the "first click" branch and unconditionally replaced m_connectionSelection, nulling it out on failure. That silently discarded a just-completed valid selection. Only replace it once the new click is confirmed to be a valid anchor; otherwise leave the existing selection (including a completed one) alone. Applied the same guard to CREATE_CONNECTIONS for consistency. Also: CREATE_CONNECTIONS's nearby-exit hover dots were drawn for every direction regardless of whether that direction already had an outgoing connection, showing candidates that would silently overwrite an existing exit. Filter to only directions with no outgoing connection yet, mirroring how SELECT_CONNECTIONS already filters to directions that do have one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
After completing a connection, CREATE_CONNECTIONS chains into a new anchor with direction ExitDirEnum::NONE (no direction picked yet, waiting for the user to hover one). But both paintNearbyConnectionPoints()'s anchor dot and paintSelectedConnection()'s ghost line called getConnectionOffset() unconditionally, which asserts(false) for ExitDirEnum::NONE -- a guaranteed crash the moment either function repainted while chained. This was latent before, but became far more likely to hit now that mouse-move always triggers a repaint. Skip drawing until a real direction is set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
mouseMoveEvent unconditionally called setSecond() on the live ConnectionSelection whenever one existed, even after the interaction had already ended (i.e. after a successful click completed a selection). Since a click's press and release aren't perfectly still, the tiny mouse movement in between kept overwriting the just-completed second endpoint with whatever was under the cursor a pixel later, silently corrupting a valid completed selection back into an invalid one -- looking like the whole selection had been cleared. Only mutate the second endpoint while an interaction is actually in progress; the repaint itself (for hover-dot tracking) still happens unconditionally. Also: ghost line color now has three states instead of two -- green for a complete valid connection, red for hovering a real room that would NOT form a valid connection, cyan (translucent) when nothing is hovered yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
A plain right-click (left button not also held) is intercepted early in mousePressEvent to fire the context menu and returns immediately, before the mode switch's "Cancel" logic for CREATE_CONNECTIONS/CREATE_ONEWAY_CONNECTIONS/SELECT_CONNECTIONS could ever run -- that cancel code was only reachable with left+right held together, an unusual chorded click. This was previously masked because an invalid left click used to wipe the selection as a side effect; once that was fixed to be a no-op, right-click became the only documented way to cancel, and it didn't actually work for a normal single right-click. Cancel the connection selection in the early right-click branch instead, alongside the existing MOVE-mode handling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
Touchscreens have no secondary button, so there was no way to cancel a pending/completed connection or room selection on touch -- only 2-finger pinch/pan was handled in touchEvent; single-finger touch fell straight through to Qt's synthesized mouse events with no equivalent of right-click. Track a single-finger touch's start position and time; if held roughly in place (within 15px) for 500ms without lifting, fire the same cancel path as right-click. A normal tap still flows through unchanged since the synthesized mouse press/release isn't touched. Factored the cancel logic (previously duplicated for connections) into a shared cancelPendingSelection(), and used it to fix the same plain-right-click-never-reaches-Cancel bug for SELECT_ROOMS that was already fixed for the connection modes -- its "Cancel" block in the mode switch was equally unreachable via a plain right-click. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHGhPpZVqQD9o39ryopvXu
1cb59c3 to
7207782
Compare
This PR removes the distracting edge-panning behavior and replaces it with modern, cross-platform navigation and tool interactions.
Key improvements:
PR created automatically by Jules for task 3155454998925703523 started by @nschimme