Fix Application Title Being Stuck After Swapping "showTerminalTitleInTitlebar" From False to True #19139
Conversation
microsoft-github-policy-service
bot
added
Issue-Bug
Techypanda
force-pushed
the
fix-use-active-terminal-title
branch
from
a590448 to
2fd692d
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -58,6 +58,8 @@ namespace winrt | |||
|
|
|||
| namespace winrt::TerminalApp::implementation | |||
| { | |||
| bool TerminalPage::_hasShownTerminalTitleInTitlebar{ false }; | |||
There was a problem hiding this comment.
ah - having a global for this may make it work poorly when there are multiple windows open. All windows are served by the same instance of WindowsTerminal.exe 🙂
There was a problem hiding this comment.
good point, didn't consider this; Let me restructure, maybe i can put this in terminal window somehow
Confirmed that if you open 2+ instances of windows terminal and try the above approach, only one window will consume the variable and fix its title. Need to find a way to broadcast to all windows to fix, will dig into this
There was a problem hiding this comment.
Hi, I have come up with a more concise/cleaner approach but its potentially slower, however, it does work with 4+ windows, could you provide guidance if this approach is against best practice (I had to introduce a getTitle to the basewindow.h) @DHowett
microsoft-github-policy-service
bot
added
Needs-Author-Feedback
Techypanda
force-pushed
the
fix-use-active-terminal-title
branch
from
2fd692d to
d0133bd
Compare
Techypanda
force-pushed
the
fix-use-active-terminal-title
branch
from
d0133bd to
3da3a52
Compare
Closes #19139 Co-authored-by: techypanda <jonathan_wright@hotmail.com>
|
Thanks so much for this! You got me wondering about how title events propagate through our application, and I came up with an alternate fix in #19212. It turns out we had all the tools to solve this, but due to organic growth the window title came from like 8 different places. Over in 19212, it all flows from one place: the I have you marked as a co-author over there too. :) |
…icrosoft#19212) We already have the logic to fall back when the setting is disabled, we just _also_ had a terrible eventing architecture and never used the `Title` getter that was smart enough to figure it out. This pull request somewhat disentangles the mess that is title handling. For example, TerminalPage had a facility to get its current title! It was never used. Once we started using it, it wouldn't work for the Settings page... because it only read the title out of the active control. Closes microsoft#12871 Closes microsoft#19139 (superseded) --------- Co-authored-by: techypanda <jonathan_wright@hotmail.com>
Summary of the Pull Request
This PR makes is so that if you are in showTabsInTitlebar = false and showTerminalTitleInTitlebar = true, then you swap showTerminalTitleInTitlebar = false. The Window title will reset back to "Terminal", this leads to a better UX of seeing Terminal instead of whatever page you were last on (in issue its Settings but really it could be anything).
References and Relevant Issues
#12871
Detailed Description of the Pull Request / Additional comments
Introduced a check on apphost when settings update, if it needs to reset title it will
Validation Steps Performed
PR Checklist