terminal
terminal copied to clipboard
Disable "Always show tabs" when "Hide the title bar" is enabled
In Global Appearance SUI, the toggle switch for "Always show tabs" is now grayed out when "Hide the title bar" is enabled (the default).
Also, the order of the settingcontainers was changed to help show the cause and effect more. See this comment.
Closes #12873
Huh, not sure why my old commits are visible here ...
Ok, I'll start working on it!
I have some questions about implementing this w/ ViewModels.
Moving ShowTitleBar...Toggled into the Appearance View Model?
I moved the event handler ShowTitlebarToggled
to the ViewModel and to bind this to the switch, I think it needs to be a static method. Because of this, editing the ViewModel properties in GlobalAppearanceViewModel::ShowTitlebarToggled
is not possible.
I think I should change ShowTitlebarToggled
to a non-static member function and bind to the switch, but IDK how to achieve this. Then, the implementation (currently commented out) should work, I think.
Is this the right approach @DHowett?
@zadjii-msft I rerequested your review so that you could get eyes on the rewrite
Thanks for the suggestions and approving 😁! But I still have some questions because I'm not seeing the result I want to achieve.
Currently, toggling ShowTabsInTitlebar doesn't immediately affect the AlwaysShowTabs toggle switch. The state of AlwaysShowTabs is changed when the Save button is clicked.
Here's the behavior I was expecting.
- User toggles ShowTabsInTitlebar from OFF to ON.
-
GlobalAppearanceViewModel::ShowTitlebarToggled
is called. -
ShowTabsInTitlebar()
returns true (because it's now on) -
AlwaysShowTabs(true)
is called. - AlwaysShowTabs toggle switch turns on and becomes disabled, thanks to XAML binding.
However, I'm observing that ShowTabsInTitlebar()
returns false, and UI of AlwaysShowTabs switch is unchanged. So the "cause and effect" is not reflected well to the user.
My code avoids the "invalid" setting where ShowTabsInTitlebar=true and AlwaysShowTabs=false because GlobalAppearanceViewModel::ShowTitlebarToggled
is called when we navigate to the GlobalAppearance page, which was what the original issue was about.
This PR may have pointed us at a incredibly subtle bug in basically all our viewmodel code. We're debugging at the moment ☺️
Based on the fact that this is actually exposing a bug that we can fix in another PR, I'm inclined to just merge this is, under the knowledge that the PR I'm about to file will fix this
Hello @zadjii-msft!
Because this pull request has the AutoMerge
label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.
p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot
) and give me an instruction to get started! Learn more here.
:tada:Windows Terminal Preview v1.16.252
has been released which incorporates this pull request.:tada:
Handy links: