feat(scroll): add consume/expel window commands (with gating/compat alignment)#151
feat(scroll): add consume/expel window commands (with gating/compat alignment)#151BarutSRB wants to merge 18 commits intoglide-wm:mainfrom
Conversation
Niri Layout
# Conflicts: # src/config.rs
- Fix animation timer: use Timer::manual() with guard to only fire while animating - Restore removed doc comments flagged in review - Apply if-let chain suggestion in reactor.rs - Use ScreenInfo struct per review suggestion - Document scroll settings individually in glide.default.toml - Add TODOs for deferred review items (configurable modifier, animation in model) - Run cargo fmt to fix CI style failures
Unify the viewport and non-viewport code paths for interactive window move hit-testing into a single loop. Previously the logic was duplicated inside an `if let Some(vp)` branch and its `else` branch. Addresses Gemini review feedback on PR glide-wm#129.
Extract the common layout-application logic from `update_layout` and `update_layout_no_anim` into a shared `apply_layout` function. A `LayoutMode` enum distinguishes between animated and immediate window placement, eliminating the near-complete duplication between the two methods. Addresses Gemini review feedback on PR glide-wm#129.
Remove the ConsumeIntoColumn and ExpelFromColumn commands in favor of using MoveNode, as suggested in PR glide-wm#129 review. Also restore comments that were inadvertently removed across layout.rs, mouse.rs, reactor.rs, and layout_tree.rs. Addresses tmandry's review feedback on PR glide-wm#129.
solve_sizes enforces a 50px minimum window size, which is appropriate for scroll layouts but breaks tree layouts with small test screens (e.g. 100x100 with 2 windows where one should be 40px after resize). Use 1.0 as the minimum for tree layouts to preserve existing behavior. Also removes a debug eprintln accidentally left in the resize handler.
apply_viewport_to_frames referenced crate::actor::app::WindowId, violating the model→actor layering rule. Make the function generic over the ID type since it only passes the value through.
SpringAnimation and ViewportState called Instant::now() directly, coupling the model layer to wall-clock time and forcing tests to use thread::sleep. Accept Instant as a parameter throughout the model API so callers in the actor layer provide the timestamp.
set_config unconditionally overwrote every scroll column's weight, discarding widths set via CycleColumnWidth or interactive resize. New columns already get their weight from visible_columns at creation time — existing columns shouldn't be touched.
For windows smaller than 2*RESIZE_EDGE_THRESHOLD, detect_edges could set both LEFT+RIGHT or TOP+BOTTOM simultaneously, causing conflicting resize deltas that cancel out. Clear both edges on an axis when the window is too small to distinguish them.
scroll_sensitivity was not validated, allowing extreme values to produce billions of traversal steps and hang the reactor. Clamp sensitivity to [0, 100], filter invalid column width presets, and cap step count to 16 as defense-in-depth.
Put scroll layout behind an experimental gate and default it to off. This adds and enforces settings.experimental.scroll.enable as the runtime gate. When the gate is off, scroll behavior is disabled: ignore scroll wheel handling for scroll layout, no-op scroll-only commands (change_layout_kind, toggle_column_tabbed, cycle_column_width), prevent default_layout_kind=scroll from activating and fall back to tree, convert active scroll layouts back to tree when the gate is disabled, and skip scroll layouts during next/prev layout cycling when gated off. Default config was updated so scroll commands are not bound by default, with opt-in examples for users who enable the experimental gate. Added tests for gate defaults and gate-off behavior.
This comment was marked as off-topic.
This comment was marked as off-topic.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@tmandry you have to try this new command. It might just be me, but I think you’ll like it. The only problem is that niri is still very buggy with more than one monitor. |
|
So this is close to what MoveNode does in the tree layout, except it "auto creates" a column if one doesn't exist in the consume case. Do you imagine this command doing the same in the tree layout case? I think this would generalize in all directions... basically if you are moving left/right it should create a column or stack; if moving up/down it would create a horizontal or tabbed node. Continuing down that path, there are basically two kinds of moves that could each be their own commands:
If a tree layout has an invariant that each level of nesting has an alternating direction, we can distinguish between a node and its parent based on the direction. This is usually the way I use it anyway, and it points to a possible simplification in the commands used to build layouts; only "split" and "group" are needed, along with these two kinds of move commands. |
|
I really love your idea of making it geenralized adopting to the layout kind and the arrow directions, this would potentially lower the commands the user needs to memorize and freeing up keys for potentially other stuff or lowering conflicts with other apps. |
|
I'll get on this tonight or tomorrow ❤️ |
|
We can merge the current behavior after a rebase. I do want to generalize but a follow-up PR works. When using this I noticed that the tabs use left/right navigation but I think stacks would be more natural, what do you think? Then you can make use of both directions and avoid cycling through all tabs in a group when using the keyboard to navigate. |
If it's the cycle command prev/next it is by design going through all the windows in order as some like to use the least amount of hot keys but we can make it ignore if column tabbed on cycle and just move to next column instead of switching tabbed windows when cycling. If it's the left/right horizontal navigation than the tabbed windows should be up/down when trying to switch them. But if you have a better idea to make the least amount of if/else logics for those commands I'm up for it. |
|
But you are right I think having cycle on tabbed columns left/right should move to next column and have an if for tabbed columns on cycle command for up/down |
|
I'm using move focus left/right commands. I think spatial commands are an important basis for how Glide is used. If people aren't familiar with vim style directions, perhaps they should use arrow keys or WASD instead. |
2 participants
Summary
Add scroll-layout
consume_or_expel_windowcommands (left/right) to move the focused window into/out of adjacent columns.What this adds
consume_or_expel_window = "left" | "right".Included compatibility/alignment fixes
While integrating this feature, this PR also includes small safety alignments so behavior stays correct:
experimental.scroll.enable = false.scroll_rootson load to preserve expected scroll-tree behavior.Files
glide.default.tomlsrc/actor/layout.rssrc/config.rssrc/model/layout_tree.rsVerification
Ran and passed locally:
cargo test consume_or_expel_window -- --nocapturecargo test rebuild_scroll_roots -- --nocapturecargo test -q scroll_wheel_is_ignored_when_scroll_gate_disabled -- --nocapturecargo test -q scroll -- --nocapture(28 passed)cargo test -q change_layout_kind -- --nocapture(1 passed)CI will run full
fmt/check/build/teston the PR.Notes
Feature remains behind existing experimental scroll gating.