Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for resizing panes with mouse #16895

Open
wants to merge 127 commits into
base: main
Choose a base branch
from

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Mar 18, 2024

Summary of the Pull Request

Adds support for resizing panes by dragging with the mouse.

ManipulationDelta is the WinUI event we're using to determine if a pane border was dragged. ManipulationDelta is kinda a wacky event in WinUI, but so is the entire Pane structure we've built. The important things to know about this PR are:

  • Leaf panes are the only panes that actually have a Border.
  • We only handle ManipulationDelta events directly on leaf panes.
  • Parent panes, however, are the only ones that actually control the position of a split. So we use a special ManipulationRequested event to bubble the request from the leaf pane that was clicked on, to the ancestor pane that actually owns the border on the side of the pane.
  • We can't just add a ManipulationDelta event handler on the border itself - the event covers all drags in the entire bounding rect of the border. So we've got to use additional logic to make sure that drags originated on a border, not just on content within the border.

Closes #992

other PRs


todo from 06-04 bugbash

  • Pane resize does not snap to cell
  • Pane resize causes a crash(? not sure if this is from that build in general, or specific to this PR)

github-merge-queue bot pushed a commit that referenced this pull request Apr 3, 2024
... technically. We still won't let it actually _be_ a pane, but now it
acts like one. It's hosted in a `SettingsPaneContent`. There's no more
`SettingsTab`. It totally _can_ be in a pane (but don't?)

## Validation Steps Performed

* Still opens the settings
* Only opens a single settings tab, or re-activates the existing one
* Session restores!
* Updates the title of the tab appropriately
* I previously _did_ use the scratchpad action to open the settings in a
pane, and that worked.

## Related PRs
* #16170
  * #16171 
    * #16172 <-- you are here 
      * #16895

Refs #997
Closes #8452
Base automatically changed from dev/migrie/f/sui-panes to main April 3, 2024 16:02
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-User Interface Issues pertaining to the user interface of the Console or Terminal Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Apr 3, 2024
Copy link
Member

@lhecker lhecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, I'm not sure whether this is the best way to go about it, in particular the bubbling of events to figure out which divider gets dragged seems somewhat... off? I feel like iterating through the tree should accomplish the same thing but be much faster, easier to debug and less complex. However, I'm not sure whether I'm in the position to suggest this as I'm somewhat afraid the Pane class may be "lacking" too much in order to support this. Still, it may be worth considering.

src/cascadia/TerminalApp/Pane.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Pane.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Pane.cpp Show resolved Hide resolved
@zadjii-msft
Copy link
Member Author

I'm not sure whether this is the best way to go about it, in particular the bubbling of events to figure out which divider gets dragged seems somewhat... off? I feel like iterating through the tree should accomplish the same thing but be much faster

Would it though? With bubbling, we start at the leaf that is at least adjacent to the border being resized, and only have to iterate O(log(n)) times upwards till at worst, the root, to find the pane that owns the border. Going top down, we'd have to check basically every pane till we found two on opposite sides of the click (right?)

@lhecker
Copy link
Member

lhecker commented Apr 3, 2024

I believe if we go top-down we can similarly decide whether descending in a sub-tree is necessary by checking if the pointer position falls into the current node's rect.

However, I estimate that a single WinRT object instantiation is roughly equivalent to iterating something in the order of dozens of Pane instances, so I believe we could just check all panes if that makes it easier to implement. The position checks are likely to be very cheap. After a divider has initially been grabbed I would suggest saving a weak reference to it so that dragging it doesn't need to walk through the tree at all. This may also double as a "is currently being dragged" state which I imagine may be useful to change the cursor, or the color of the divider, etc., in the future.

github-merge-queue bot pushed a commit that referenced this pull request Apr 5, 2024
…splitPane`, `newTab` (#16914)

This changes `NewTabArgs`, `SplitPaneArgs`, and `NewWindowArgs` to
accept a `INewContentArgs`, rather than just a `NewTerminalArgs`. This
allows a couple things:
* Users can open arbitrary types of panes with the existing `splitPane`,
`newWindow` actions, just by passing `"type": "scartchpad"` (for
example). This is a lot more flexible than re-defining different
`"openScratchpad"`, `"openTasksPane"`, etc, etc actions for every kind
of pane.
* This allows us to use the existing machinery of session restore to
also restore non-terminal panes.

The `type` property was added to `newTab`, `splitPane`, `newWindow`.
When omitted, we still just treat the json as a blob of NewTerminalArgs.

There's not actually any other kinds of `INewContentArgs` in this PR
(other than the placeholder `GenericContentArgs`). In
[`dev/migrie/fhl/md-pane`](dev/migrie/f/tasks-pane...dev/migrie/fhl/md-pane),
I have a type of pane that would LOVE to add some args here. So that's
forward-thinking.

There's really just two stealth types of pane for now: `settings`, and
`scratchpad`. Those I DON'T have as constants or anything in this PR.
They probably should be? Though, I suspect around the time of the tasks
& MD panes, I'll come up with whatever structure I actually want them to
take.

### future considerations here

* In the future, this should allow extensions to say "I know how to host
`foo` content", for 3p content.
* The `wt` CLI args were not yet updated to also accept `--type` yet.
There's no reason we couldn't easily do that.
* I considered adding `ICanHasCommandline` to allow arbitrary content to
generate a `wt` commandline-serializable string. Punted on that for now.


## other PRs
* #16170
  * #16171 
    * #16172 
      * #16895 
      * #16914 <-- you are here 

Closes #17014
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Played with the branch. Feels solid!

@DHowett
Copy link
Member

DHowett commented May 2, 2024

Sorry we didn't land this for 1.21 - I was looking over Leonard's concerns and I'm wondering if there's a nice middle ground we can strike!

@zadjii-msft
Copy link
Member Author

Just sticking this in here because I'm sure I'll lose it in this notebook after the next couple weeks:
image

I had tried to mock out what Leonard's approach would be. I was pretty sure there wasn't a middle approach between the two, but doing it top-down would be entirely different.

I'll probably come back to try this after Build

@zadjii-msft zadjii-msft added this to the Terminal v1.22 milestone May 31, 2024
@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Jul 17, 2024
@zadjii-msft
Copy link
Member Author

discussion notes:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panes should be resizable with the mouse
5 participants