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

Only show cross-hairs when terminal has focus #1772

Merged
merged 4 commits into from Nov 26, 2018
Merged

Conversation

whydoubt
Copy link
Contributor

Fixes #1767

@jerch
Copy link
Member

jerch commented Nov 8, 2018

@whydoubt This is not working for me, I dont get crosshair cursor at all now for column selection. (Ubuntu 16, tested with Firefox and Chrome) Any clue whats going on?

@whydoubt
Copy link
Contributor Author

whydoubt commented Nov 13, 2018

No, I don't understand what the problem might be. All of these worked for me:

  • Chrome 69 on Windows 7
  • IE11 on Windows 7
  • Firefox 58 on Fedora 27
  • Chromium 63 on Fedora 27

Using dev tools to watch the classes applied to the child of terminal-container, you should see 'focus' added if you click on the terminal and removed if you click outside. When focused, pressing and releasing the Alt key should add and remove 'column-select'. Alt-Tab should add 'column-select' on Alt, then remove 'focus' on Tab. Cross-hairs should be visible when and only when both 'focus' and 'column-select' are applied. FWIW, with Chrome on Windows 7 I did find that the cursor would not change until I moved the mouse.

@jerch
Copy link
Member

jerch commented Nov 14, 2018

Ah yepp my bad, the css file was not refreshed. It kinda partially solves the issue #1767 - after ALT+TAB with a different focus window in between it works, but it still shows the crosshair if I tab through all windows back to Chrome. Then I have to click in Chrome outside of the terminal div to get rid of the crosshair. I think thats what was meant by the issue? Any idea how to solve this? (Might also be windowmanager dependent, using xfce here.)

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Hmm, I'm not so sure we want to rely on focus here. Take VS Code as an example, if you have the editor focused and hold alt, the crosshair should still show up. I think a better fix might be to clear the class when the document blur event fires?

@@ -139,7 +139,7 @@
cursor: pointer;
}

.xterm.xterm-cursor-crosshair {
Copy link
Member

Choose a reason for hiding this comment

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

xterm-cursor-crosshair aligns with the naming of xterm-cursor-pointer, not sure we want to change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current alignment of the names re-enforces the false impression that the two are strongly correlated, and is part of why the name should change. At some point xterm-cursor-pointer should be replaced with something more semantic as well (maybe something like linkified).

@whydoubt
Copy link
Contributor Author

whydoubt commented Nov 20, 2018

@Tyriar : not sure what your point regarding VS Code is, because if I understand you correctly, it does not act that way now, and changing it would depend on changing VS Code. Regarding clearing the column-select class on blur, I will look into that.
@jerch : With dev tools, can you see what is happening in terms of the applied classes as I described in an earlier comment?

@jerch
Copy link
Member

jerch commented Nov 23, 2018

@whydoubt Yepp the classes are applied correctly for tabbing in and out with a different focused window in between, but not if I tab through all right back to chrome. As I said Im not sure if thats fixable at all as it might be related to the way the window manager handles/steals focus.

@whydoubt
Copy link
Contributor Author

@jerch It appears to affect all browsers I have on Fedora—with GNOME Shell handling window management. I see that a keydown event for the first Alt is received, but no further events. The window manager ostensibly swallows all further keyups/keydowns, and no focus/blur events get fired either. There may not be a solution to this one, unless we can find some event that does get fired.
BTW, pressing any key once you have returned should clear this, as this forces a keydown event.

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

not sure what your point regarding VS Code is, because if I understand you correctly, it does not act that way now

Opps, you're right 😅. While this is certainly not perfect, this definitely improves the situation and fixes the underlying bug. Thanks @whydoubt!

@Tyriar Tyriar merged commit 68d5ce9 into xtermjs:master Nov 26, 2018
@jerch
Copy link
Member

jerch commented Nov 26, 2018

@whydoubt Yeah I think the situation with the window manager cannot be improved by normal means here (means from within the browser). So the partial solution is good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants