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

Pane movement breaks showCloseButton=never #15902

Closed
regnaio opened this issue Aug 30, 2023 · 2 comments · Fixed by #15914
Closed

Pane movement breaks showCloseButton=never #15902

regnaio opened this issue Aug 30, 2023 · 2 comments · Fixed by #15914
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Area-User Interface Issues pertaining to the user interface of the Console or Terminal In-PR This issue has a related PR 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.

Comments

@regnaio
Copy link

regnaio commented Aug 30, 2023

Windows Terminal version

1.17.11461.0

Windows build number

Version 10.0.19045.3324

Other Software

No response

Steps to reproduce

Add the following to settings.json (in %LOCALAPPDATA%\Packages\Microsoft.WindowsTerminal_8wekyb3d8bbwe\LocalState\settings.json):

"theme": "foo",
"themes": [
  {
    "name": "foo",
    "tab": {
      "showCloseButton": "never"
    }
  }
]

Run in Command Prompt:

wt -M -p "Ubuntu-20.04" ; ^
	split-pane -p "Ubuntu-20.04" ;

Observe that the first tab has an "X" on the right that you can click to close the tab. But, tab.showCloseButton: never should have hidden this "X". The split-pane line is causing this bug. If you remove it, i.e.

wt -M -p "Ubuntu-20.04" ;

then the tab no longer has the "X", as expected

For related discussion:

Expected Behavior

The "X" icon should not be present on a Windows Terminal tab if tab.showCloseButton: never

Actual Behavior

The "X" icon is present on a tab if using split-pane in wt


Edit (lhecker): Any movement between two panes makes the buttons reappear.

@regnaio regnaio added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 30, 2023
@lhecker lhecker added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. Priority-3 A description (P3) labels Aug 30, 2023
@lhecker lhecker added this to the Terminal v1.20 milestone Aug 30, 2023
@lhecker lhecker removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 30, 2023
@DHowett DHowett changed the title "X" on tab is never hidden when using wt split-pane Pane movement breaks showCloseButton=never Aug 30, 2023
@DHowett
Copy link
Member

DHowett commented Aug 30, 2023

Thanks for filing! We found a more general issue with showCloseButton thanks to you 😄

@zadjii-msft
Copy link
Member

Oh well that'll do it:

  • TerminalPage::_updateAllTabCloseButtons is where we apply TabViewItem().IsClosable based off the theme
  • Presumably, TerminalTab::_RecalculateAndApplyReadOnly fires later, which then updates the tab's own item's IsClosable based on read-only state only.

zadjii-msft added a commit that referenced this issue Aug 31, 2023
`TerminalTab::_RecalculateAndApplyReadOnly` didn't know about whether a tab should be closable or not, based on the theme settings. Similarly (though, unreported), the theme update in `TerminalPage::_updateAllTabCloseButtons` didn't really know about readonly mode.

This fixes both these issues by moving responsibility for the tab close button visibility into `TabBase` itself.

Closes #15902
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Aug 31, 2023
zadjii-msft added a commit that referenced this issue Sep 6, 2023
…de (#15914)

`TerminalTab::_RecalculateAndApplyReadOnly` didn't know about whether a
tab should be closable or not, based on the theme settings. Similarly
(though, unreported), the theme update in
`TerminalPage::_updateAllTabCloseButtons` didn't really know about
readonly mode.

This fixes both these issues by moving responsibility for the tab close
button visibility into `TabBase` itself.

Closes #15902
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Sep 6, 2023
DHowett pushed a commit that referenced this issue Sep 22, 2023
…de (#15914)

`TerminalTab::_RecalculateAndApplyReadOnly` didn't know about whether a
tab should be closable or not, based on the theme settings. Similarly
(though, unreported), the theme update in
`TerminalPage::_updateAllTabCloseButtons` didn't really know about
readonly mode.

This fixes both these issues by moving responsibility for the tab close
button visibility into `TabBase` itself.

Closes #15902

(cherry picked from commit 2f41d23)
Service-Card-Id: 90379173
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Area-User Interface Issues pertaining to the user interface of the Console or Terminal In-PR This issue has a related PR 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants