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.

Sign up for GitHub

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

Jump to bottom

Settings reload forces cursor to be visible/blinking even when terminal is out of focus #7571

Closed
vefatica opened this issue Sep 8, 2020 · 3 comments
Closed

Settings reload forces cursor to be visible/blinking even when terminal is out of focus #7571

vefatica opened this issue Sep 8, 2020 · 3 comments
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal Backlog

Comments

@vefatica
Copy link

vefatica commented Sep 8, 2020

Environment

Microsoft Windows 10 Pro for Workstations
10.0.18363.1016 (1909)
Windows Terminal Preview
Version: 1.3.2382.0

Steps to reproduce

From the dropdown, choose "Settings". Make any change to settings.json and save.

Expected behavior

None

Actual behavior

Cursor becomes visible in WT though it has neither foreground nor input focus.

That's actually quite disconcerting ... to be working in an editor while WT (inactive and on another monitor) is blinking at me
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 8, 2020
@DHowett DHowett changed the title Cursor on when WT does not have Foreground/Focus Settings reload forces cursor to be visible/blinking even when terminal is out of focus Sep 9, 2020
@DHowett DHowett added good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 9, 2020
@DHowett DHowett added this to the Terminal Backlog milestone Sep 9, 2020
@DHowett DHowett added the Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) label Sep 9, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 9, 2020
@DHowett
Copy link
Member

DHowett commented Sep 9, 2020

Thanks! Tagged.

@vefatica vefatica mentioned this issue Sep 11, 2020
Open
@Don-Vito Don-Vito mentioned this issue Oct 11, 2020
Merged
6 tasks
@ghost ghost added In-PR This issue has a related PR and removed In-PR This issue has a related PR labels Oct 11, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Needs-Tag-Fix Doesn't match tag requirements labels Oct 13, 2020
DHowett pushed a commit that referenced this issue Oct 19, 2020
@Don-Vito @DHowett
7571: do not activate terminal window upon settings modificaion (#7887)
e3edf18 
Took this as an easy starter. The method IslandWindow::SetAlwaysOnTop is
triggered once terminal settings are reloaded (in
TerminalPage::_RefreshUIForSettingsReload flow). This method calls
SetWindowPos without SWP_NOACTIVATE. As a result the window gets
activated, the focus is set and the cursor starts blinking.

Added SWP_NOACTIVATE in all SetWindowPos calls from IslandWindow and
NoClientIslandWindow (where it was missing). Please let me know if this
is an overkill - it is not required to fix the issue, however seems a
good practice, that might help if we decide to apply more settings
immediately.

## Validation Steps Performed
* Only manual testing - please guide me to the relevant UT framework, if
  exists.
* Trying to reproduce this with VS attached doesn't work - the window
  gets the focus in any case.
* Tested as a standalone application, by modifying different settings
  (and comparing the results before and after the fix).
* Checked with Spy++ that no WM_ACTIVATE / WM_SETFOCUS is thrown upon
  settings modification
* Applied terminal resizing, toggling full screen and focus mode to
  check no regression was introduced.

Closes #7571

(cherry picked from commit cb732a4)
@ghost
Copy link

ghost commented Nov 11, 2020

🎉This issue was addressed in #7887, which has now been successfully released as Windows Terminal v1.4.3141.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Nov 11, 2020

🎉This issue was addressed in #7887, which has now been successfully released as Windows Terminal Preview v1.5.3142.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal Backlog
Development

No branches or pull requests
2 participants
@DHowett @vefatica