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

Add tab.closeButton: activeOnly #13672

Closed
zadjii-msft opened this issue Aug 4, 2022 · 1 comment · Fixed by #15237
Closed

Add tab.closeButton: activeOnly #13672

zadjii-msft opened this issue Aug 4, 2022 · 1 comment · Fixed by #15237
Labels
Area-Theming Anything related to the theming of elements of the window 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. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone

Comments

@zadjii-msft
Copy link
Member

zadjii-msft commented Aug 4, 2022

TabView is a part of WinUI, and that code is available over at https://github.com/microsoft/microsoft-ui-xaml/

I want to know if it's even possible on the current version of TabView to actualize what that enum would represent?

Hm.

if (const auto theme = _settings.GlobalSettings().CurrentTheme())
{
const auto visibility = theme.Tab() ? theme.Tab().ShowCloseButton() : Settings::Model::TabCloseButtonVisibility::Always;
for (const auto& tab : _tabs)
{
switch (visibility)
{
case Settings::Model::TabCloseButtonVisibility::Never:
tab.TabViewItem().IsClosable(false);
break;
case Settings::Model::TabCloseButtonVisibility::Hover:
tab.TabViewItem().IsClosable(true);
break;
default:
tab.TabViewItem().IsClosable(true);
break;
}
}
switch (visibility)
{
case Settings::Model::TabCloseButtonVisibility::Never:
_tabView.CloseButtonOverlayMode(MUX::Controls::TabViewCloseButtonOverlayMode::Auto);
break;
case Settings::Model::TabCloseButtonVisibility::Hover:
_tabView.CloseButtonOverlayMode(MUX::Controls::TabViewCloseButtonOverlayMode::OnPointerOver);
break;
default:
_tabView.CloseButtonOverlayMode(MUX::Controls::TabViewCloseButtonOverlayMode::Always);
break;
}
}

Note

Walkthrough

  • We could take that, turn that into a method.
  • Call that method every time the active tab changes...
  • ... or the set of tabs changes.
  • Get rid of all the calls to _updateTabCloseButton, cause that only updates a single tab.
  • Add logic in that method
    • Inside the loop, if the mode is ActiveOnly, and the tab is active, then IsClosable(true). else false
    • Outside the loop, set CloseButtonOverlayMode to Auto, probably a sensible default.

That sounds like it would work on paper. Wanna file a PR? 😄

Originally posted by @zadjii-msft in #3335 (comment)

@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 Aug 4, 2022
@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. good first issue This is a fix that might be easier for someone to do as a first contribution Area-Theming Anything related to the theming of elements of the window labels Aug 5, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 5, 2022
@zadjii-msft zadjii-msft added this to the Backlog milestone Aug 5, 2022
@shourya5
Copy link

shourya5 commented Aug 7, 2022

I have a question,for:

Call that method every time the active tab change
... or the set of tabs changes.

what would be the best way to implement this(as in check which tab(s) are active)?

@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 12, 2022
@zadjii-msft zadjii-msft moved this to Should be written in Terminal Walkthroughs Feb 6, 2023
@zadjii-msft zadjii-msft moved this from Should be written to Walkthrough in issue in Terminal Walkthroughs Feb 6, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Apr 25, 2023
@github-project-automation github-project-automation bot moved this from Walkthrough in issue to Done in Terminal Walkthroughs May 3, 2023
microsoft-github-policy-service bot pushed a commit that referenced this issue May 3, 2023
This MR introduces `activeOnly ` for the `showCloseButton` theme option
causing the close button only to appear on the active tab.

This is more or less following the approach explained here
https://github.com/orgs/microsoft/projects/686/views/2?pane=issue&itemId=19775774
which indeed just works 😄 .

You notice when switching theme the close buttons is back on all tabs
again as well.

Closes #13672

I didn't check specific unit tests for this. I hope by making this MR
the pipeline will show if I broke something. Or just let me know if you
want me to add something specific for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Theming Anything related to the theming of elements of the window 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. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants