feat!: toggle snap instead of hold to snap#52
Open
webbertakken wants to merge 6 commits intoBasGeertsema:mainfrom
Open
feat!: toggle snap instead of hold to snap#52webbertakken wants to merge 6 commits intoBasGeertsema:mainfrom
webbertakken wants to merge 6 commits intoBasGeertsema:mainfrom
Conversation
When the new 'stickySnap' setting is enabled, a single press of the activator (secondary mouse button or modifier key) latches snapping on for the rest of the drag. Primary mouse button release commits the snap; Escape cancels. Default is off, so existing users get no behavior change. Why a restart-grab dance is needed ---------------------------------- Muffin's MOVING grab terminates on any secondary-button event while the primary button is held (confirmed empirically, and in src/core/window.c::meta_window_handle_mouse_grab_op_event). A Cinnamon extension cannot veto that from JS because Muffin consumes the event via clutter_event_add_filter, which runs before any stage captured- event or later filter added from JS. The workaround: in grab-op-end we check whether the primary button is still physically down and, if so, re-issue a MOVING grab on the same window via global.display.begin_grab_op(win, MOVING, false, true, 1, 0, time, root_x, root_y). Passing explicit root_x/root_y (vs. the window-level begin_grab_op, which warps the cursor to the window centre) preserves the grip point between cursor and frame. Implementation -------------- - settings-schema.json: new boolean 'stickySnap', default false. - node_tree.js: SnappingOperation gains a #sticky latch plus setSticky()/isSticky accessors. onMotion latches #sticky when a normal activation is observed under sticky mode and treats the latch as a forced snappingEnabled=true thereafter. cancel() does NOT clear the latch (ownership is setSticky/destroy only), so multi-monitor cleanup in onMotion can call cancel without nuking the per-drag state. - window-snapper.js: new activateSticky()/deactivateSticky() and #forceMotionUpdate() that populate the overlay from current pointer state without needing mouse motion. - application.js: grab-op-begin distinguishes fresh drags (build snappers, install Escape filter) from restart cycles (just refresh window ref). grab-op-end schedules a GLib idle restart whenever b1Held, stickySnap, no cancel flag, and under a per-drag cap of MAX_RESTARTS_PER_DRAG=100. No time-based cooldown: Muffin emits grab-end for both the RMB press AND its release within tens of milliseconds, and a cooldown would cause one of them to fall through to finalize(), committing a snap prematurely. - Escape handler installed via Clutter.event_add_filter for the lifetime of a sticky drag. On Escape we set #dragCancelled, call deactivateSticky() on all snappers, and swallow the event. Incidental fix -------------- SnappingOperation.onMotion now calls cancel() when the pointer is outside this monitor's layout rect. Previously an off-monitor snapper silently returned notHandled() and retained stale highlights, so on LMB release every snapper's finalize() could fire, producing phantom snaps on the wrong monitor. The bug existed in hold mode too but was masked by the activation toggling highlights off; sticky mode made it visible.
Application had accumulated five per-drag fields (#currentDragWindow,
#pendingRestartId, #restartCount, #dragKeyFilterId, #dragCancelled),
three helper methods (#runRestart, #installEscapeFilter,
#removeEscapeFilter), and a large amount of state-mutation logic spread
across its grab-op-begin / grab-op-end handlers. Together those
formed an implicit state machine for keeping the drag alive across
Muffin's grab tear-downs, plus Escape cancellation, plus sticky latch
activation.
This change lifts that state machine into a dedicated DragSession
class in drag-session.js, with a small four-method public API:
new DragSession({ window, layoutFor, options })
session.onGrabRestart(window) // new grab-begin = our own restart
session.tryRestart() // grab-end -> restart or finish?
session.finish() // commit (or skip) + tear down
Application becomes a thin dispatcher with one drag-related field
(#dragSession) and two short handlers. Settings are snapshotted at
session construction so mid-drag changes can't corrupt in-flight
state. No behavior change.
Previously, zones would only appear after the user released the
secondary mouse button (Muffin tears down the MOVING grab on RMB
release on this build, not on press). If the user pressed and held
RMB without moving a pixel, there was no trigger for activateSticky:
the grab-restart dance only fires on grab-op-end, and position-changed
only fires on actual window motion. Button events are consumed by
Muffin's own event filter during a MOVING grab, so a Clutter event
filter cannot observe the press directly.
Fix: while a sticky drag is waiting for its first activation, poll
the pointer at 60 Hz via GLib.timeout_add and re-run onMotion on
every snapper. The instant RMB (or the configured modifier) is
pressed, onMotion sees the state bit and latches sticky. The poller
self-terminates as soon as any snapper reports isSticky, so there's
no ongoing cost once the overlay is up.
WindowSnapper exposes a small public surface to support this:
- isSticky (delegates to SnappingOperation.isSticky)
- refreshFromPointer() (the motion-re-eval logic, moved out of
#onWindowMoved which is now a one-liner delegate)
SnappingOperation.isSticky was re-added after a previous cleanup
removed it as unused; DragSession's poll-stop condition needs it.
Previous commits attempted to wire Escape to cancel a sticky drag
via a Clutter event filter and/or a Main.keybindingManager hotkey.
Neither actually fires during a MOVING grab:
- Muffin's event filter is registered before any extension's, so a
JS-added Clutter event filter never sees the Escape key press.
- Keybindings registered via Main.keybindingManager reach Muffin's
process_event, but process_event filters out every non-workspace
action while mouse_grab_move is true (keybindings.c, around the
'mouse_grab_move && !is_workspace_action' guard).
On top of that, Muffin's built-in Escape handler unconditionally
teleports the window back to its grab-initial position before ending
the grab, and that cannot be prevented from the extension layer.
Rather than ship half-working cancel code, remove it entirely.
Escape during a sticky drag now just triggers Muffin's default
behaviour (window back to drag-start, grab ends). The user can re-
drag from there. If we ever revisit this, a position-based heuristic
to at least hide the zones after Muffin's Escape teleport is
documented in the git history.
Removed:
- DragSession #installEscapeFilter / #removeEscapeFilter / #onEscape
- DragSession #escapeFilterId field + call sites
- WindowSnapper.deactivateSticky() (had no other callers)
- Escape mention in DragSession's doc comment
Remove the stickySnap opt-in setting. Snapping activation is now
always sticky: a single press of the configured activator (secondary
mouse button or modifier key) latches the snap overlay on for the
rest of the drag; LMB release commits.
Removed:
- settings-schema.json 'stickySnap' entry
- Application.#snapshotDragOptions 'stickySnap' field
- DragSession's 'if (options.stickySnap)' gate around the
activation poller (poller always starts)
- DragSession.tryRestart's '!this.#options.stickySnap' check
- WindowSnapper ctor arg 'stickySnap' and #stickySnap field
- SnappingOperation ctor arg 'stickySnap' and #stickySnap field
Simplified SnappingOperation.onMotion: the latch now triggers on any
normal activation (no mode gate), and snappingEnabled reduces to
this.#sticky (the latch).
Tooltips for 'enableSnappingModifiers' and 'activateWithNonPrimaryButton'
updated from 'holding' to 'pressing' to reflect the new behavior.
BREAKING CHANGE: users who relied on classic hold-to-snap UX
('snap only commits while the activator is held at LMB release')
will see the overlay stay on after the activator is released and a
snap commit on the next LMB release. The Escape key-cancel provided
by Muffin (window returns to drag-start, grab ends) is the only way
to abort from inside a drag.
This was referenced Apr 22, 2026
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
1 participant
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.
What changed:
Technical:
DragSessionclass to encapsulate all the per-drag state.Additional context:
grab-op-endif primary button is still pressed, we immediately restore the grab, so that the window is still held as dragging. This is an uninterrupted flag that. The measure is invisible to the user.P.S.: