Skip to content

Select and Hand toolbar buttons are checkable placeholders with no behavior #103

Closed

Description

@mberrys

Select and Hand toolbar buttons are checkable placeholders with no behavior

ShellToolBar.qml:121-135 presents Select and Hand as checkable tool buttons, but neither has an onClicked handler, neither is bound to activeTool, and the two can be checked independently. Pressing them changes nothing about how the canvas responds.

Wiring them to the existing InteractionController::setActiveTool is not a sufficient fix, and should not be attempted on its own.

Why the obvious fix is wrong

InteractionController::m_activeTool is currently write-only. A repo-wide search for activeTool() returns two call sites, both in LoopLibQuick/sources/loopcanvasitem.cpp (lines 190-197), and both merely pass the string back to setActiveTool for an equality comparison.

Inside the interaction layer the member is assigned at LoopLibInteraction/sources/interactioncontroller.cpp:111 and never read to branch any behavior. handlePointerPress (lines 226-268) routes pan to m_panButton (default middle mouse), and routes selection and drag off pointer button, modifiers, and InteractionTargetKind — none of which consult the active tool.

Exposing the setter to QML would therefore make two dead controls look live while changing no observable behavior. That is a worse defect than the current honest-but-inert one, because the operator loses the ability to tell that the feature is absent.

What a real fix requires

  1. An agreed tool vocabulary. Tool IDs are currently free-form strings. The only test uses "measure" (UnitTests/tst_interactioncontrollertest.cpp:448), which no production path sets. Nothing in docs/ defines the vocabulary.
  2. Hand/select pointer semantics. Hand must claim left-button drag for panning and suppress selection and drag initiation; Select must retain today's behavior. Today m_panButton is middle mouse, so left-drag always reaches the select path.
  3. Cancel-on-change is already correct and must be preserved. setActiveTool calls cancelActive(InteractionCancelReason::ToolChanged) (issue X00-02 — Re-file parked legacy expansion candidates with value cases #141 AC3), so an in-flight drag must not leave a half-applied transform behind.

Boundary and authority

This is a missing feature, not a defect, and it needs a design decision before code:

  • Behavior changes land in LoopLibInteraction/**, which agent-policy.json maps to the interaction boundary with its own proof lane (UnitTestsInteractionController, UnitTestsInteractionBoundary, UnitTestsHitTestSource, UnitTestsOverlayFrame).
  • The QML side (LoopEditor/qml/**, ProductQuickAccessibilitySmoke/**) is in the quick boundary.

Acceptance criteria

  • A tool vocabulary is defined in a contract doc, not implied by control labels.
  • Hand pans on left-drag and does not change selection.
  • Select retains current selection and drag behavior.
  • Switching tools mid-drag cancels the drag with ToolChanged and commits nothing.
  • The toolbar reflects the real activeTool as a single-select choice, so Select and Hand cannot both appear checked.
  • Every changed QML file is byte-identical under ProductQuickAccessibilitySmoke/qml/.
  • New C++ carries failing tests first, then the mapped interaction and quick lanes pass.

Related

The EditorHost plumbing that would carry this (activeTool property plus a setActiveTool invokable routing through the attached canvas) was prototyped and reverted rather than committed, because on its own it is a no-op. It is straightforward to re-derive once the semantics above exist.

Refs #141

L14 adoption (2026-10-01)

Tracked by L14 Interaction Performance #179 and its long-session qualification #187. This is a cross-link, not a reassignment or relaxation of the tool-vocabulary, cancellation, interaction proof or original issue's ownership and acceptance criteria.

Activity

  1. mberrys commented on Oct 7, 2026

    @mberrys
    ContributorAuthor

    Fixed on dev by da4e63f ("Give Select and Hand real tool semantics"). The tool vocabulary is in docs/INTERACTION_CONTRACT.md, and Hand/Select pointer semantics are in InteractionController. Toolbar buttons are bound to host.activeTool in an exclusive group. Coverage is listed in docs/QUICK_CANVAS_EDITOR_GAPS.md section 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    Promotion stage

    Stable present

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions