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

Switch to the I-beam cursor when hovering over the terminal #5028

Closed
wants to merge 7 commits into from

Conversation

DHowett-MSFT
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT commented Mar 20, 2020

This commit makes us use the I-beam cursor when the user hovers over the
terminal, unless mouse mode is enabled. I've also plumbed up a bunch
of events so that:

  • If mouse mode is toggled while hovering, the cursor will switch to
    the arrow if it's on or the I-beam if it's off.
  • If you hold down shift to suppress mouse mode, the cursor will switch
    back to the I-beam.

Fixes #1441.

Detailed Description of the Pull Request / Additional comments

It turns out that we're in violation of the Windows human interface guidelines, so that's good.

It also turns out that you cannot set the cursor type of a control -- you actually have to actively set and reset the cursor when the pointer enters or exits.

terminal_mouse
my selection color is bright red to differentiate it from vim's mouse mode selection

This commit makes us use the I-beam cursor when the user hovers over the
terminal, *unless* mouse mode is enabled. I've also plumbed up a bunch
of events so that:

* If mouse mode is _toggled_ while hovering, the cursor will switch to
  the arrow if it's on or the I-beam if it's off.
* If you hold down shift to suppress mouse mode, the cursor will switch
  back to the I-beam.

Fixes #1441.
@DHowett-MSFT DHowett-MSFT marked this pull request as ready for review March 20, 2020 22:38
@zadjii-msft
Copy link
Member

On a scale of yes to no, could this be made a setting (in a future PR) so I could disable this? Should it be a global or a profile setting?

@DHowett-MSFT
Copy link
Contributor Author

In a future PR, it could probably be made a setting -- global, though.

@mdtauk
Copy link

mdtauk commented Mar 20, 2020

If it is implemented, it should work with the new Beam cursor options.

image

@DHowett-MSFT
Copy link
Contributor Author

@mdtauk that is not an i-beam mouse cursor. whatever that is works with Terminal as of v0.10 (or v0.11? which is not yet out; it merged recently.)

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

I'm okay with this, on one condition. I'd like to have a plan for what we're going to call the setting to disable this before we ship this, cause I'm a pointer kind of guy 😝

{
// If the current focused element is a child element of searchbox,
// we do not send this event up to terminal
if (_searchBox && _searchBox->ContainsFocus())
Copy link
Member

Choose a reason for hiding this comment

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

Presumably this bit would go away with #4748?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep!

src/cascadia/TerminalControl/TermControl.cpp Show resolved Hide resolved
src/cascadia/TerminalControl/TermControl.cpp Show resolved Hide resolved
if (_oldCursor) // if we have an active cursor transition
{
auto coreWindow = Window::Current().CoreWindow();
coreWindow.PointerCursor(_CanSendVTMouseInput() ? _pointerCursor : _textCursor);
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a helper method that's just "set the PointerCursor to the correct cursor for our current mouse input mode"?

@DHowett-MSFT
Copy link
Contributor Author

So, I think this is your setting:

image

Cheekiness aside, I'm not averse to speccing a global setting that configures how the cursor works -- but it will be a case of users opting for less expressiveness/fewer features, so we should almost certainly measure its use.

@DHowett-MSFT
Copy link
Contributor Author

This is in selfhost. Give it a feel.

@zadjii-msft
Copy link
Member

  • The cursor remains an I-beam when I mouse over the tabs
  • If I open tmux (with mouse mode) in one tab, then hover another tab, the cursor switches back to the I-beam, and stays in an I beam even if I click inside tmux

@zadjii-msft
Copy link
Member

showerthought: what if we just always turned the cursor back into an arrow, then let the new control change it to whatever they want? Presumably one control's PointerExited is fired before the next control's PointerEntered, right? Or does that cause flickering?

Additionally: Are we planning on putting this is 1.0?

@DHowett-MSFT
Copy link
Contributor Author

DHowett-MSFT commented Apr 1, 2020

not sure about 1.0. My concern with always setting Arrow is that what if they're hovering with a pen? I dunno, I kinda wanna ask AustinL.

@bropines

This comment was marked as off-topic.

@DHowett-MSFT

This comment was marked as off-topic.

@egmontkob
Copy link

So this is about the mouse pointer (mouse cursor), and not the text insertion cursor. I legit had to read the thread halfway through to realize it.

Don't you guys always confuse these two? I do. Similar names, the word "cursor" overloaded for them, and both possibly being of "I-beam" shape.

My preference is to say "mouse pointer" for, well, that one; without using the word "cursor". Do you guys have a preferred terminology?

@bropines
Copy link

bropines commented May 3, 2020

@bropines похоже, что вы бежите в #4448

Thank you very much. I wouldn't have guessed it myself

@DHowett-MSFT
Copy link
Contributor Author

@egmontkob yep, this is about the mouse pointer. I should endeavour to be better about saying "pointer" when I mean the thing the user points with. 😄

Alas, overloaded terms are pervasive in our industry. They're so hard to escape.

@DHowett-MSFT
Copy link
Contributor Author

(And yeah, we get them mixed up all the time.)

Comment on lines +171 to +172
winrt::Windows::UI::Core::CoreCursor _textCursor;
winrt::Windows::UI::Core::CoreCursor _pointerCursor;
Copy link
Member

Choose a reason for hiding this comment

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

(Correct me if I'm wrong) It looks like you only set these in the constructor. Why not just make them constexpr in the namespace?

Copy link
Member

Choose a reason for hiding this comment

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

they're winrt objects. it sucks.

@jsoref
Copy link
Contributor

jsoref commented May 28, 2020

The conflicts shouldn't be hard to resolve whitelist was renamed to expect -- If you need help, I can help on Sunday.

@zadjii-msft zadjii-msft removed this from the Terminal v1.x milestone Aug 18, 2020
@angelog0
Copy link

All the decent Terminals I know have this behavior: when the mouse pointer is over them it changes into a sort of I. This makes selecting text more intuitive and practical...

@DHowett
Copy link
Member

DHowett commented Sep 11, 2020

And you are commenting on this open pull request that does exactly this so as to move it along?

@microsoft-github-updates microsoft-github-updates bot changed the base branch from master to main October 22, 2020 00:27
@ghost ghost added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Oct 22, 2020
@dufferzafar
Copy link

Is this still being worked on?

@DHowett
Copy link
Member

DHowett commented Mar 10, 2021

Not actively.

@DHowett DHowett marked this pull request as draft March 10, 2021 00:29
@github-actions

This comment has been minimized.

@ghost ghost dismissed a stale review via 34f83a6 November 2, 2021 13:46
@miniksa
Copy link
Member

miniksa commented Jan 14, 2022

Closing as it's super stale, but not deleting the branch in case someone wants to dig it up again one day.

@zadjii-msft
Copy link
Member

To add some notes that never made it here a few years back:

I believe there was no good way for us to set the mouse pointer for a single control in XAML. A control could change the cursor in PointerEntered, but IIRC there's a race between PointerEnter's and PointerExits. So when you've got an app that's got all sorts of mixed UI elements, some that want the cursor to be an I-beam and some that want the arrow, well now you're stuck. As you hover over different elements, they'll fight over who's actually setting the cursor to what.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mouse pointer should be text and not arrow one