microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.65k stars 8.32k forks source link

Remaining tabs are not resized when closing tabs #9822

Open DavidZidar opened 3 years ago

DavidZidar commented 3 years ago

Windows Terminal version (or Windows build number)

1.8.1032.0

Other Software

No response

Steps to reproduce

Open a bunch of tabs, close all but a few with the keyboard.

Expected Behavior

I expect tabs to be restored to their original size when the space allows for it.

Actual Behavior

Tabs remain compacted indefinitely. The only way to get the tabs back to their expected size seems to be to grab the mouse and hover over the tab bar and then move the mouse off the tab bar again.

DavidZidar commented 3 years ago

I understand the tabs remain fixed in size until the mouse leaves the tab bar like what was mentioned in #8030, but this makes no sense when closing tabs with the keyboard or even when just exiting shells while the mouse is nowhere near the tab bar.

DHowett commented 3 years ago

We suspect that this is an issue with MUX 2.5 :|

DavidZidar commented 3 years ago

Should I report this issue at microsoft/microsoft-ui-xaml or hold off until you are sure?

DHowett commented 3 years ago

Oh, sorry -- I'll move this issue report over there. I wasn't very clear on that. :)

DavidZidar commented 3 years ago

Thank you! And thanks for Windows Terminal, it's awesome! :)

zadjii-msft commented 3 years ago

I've moved this upstream to /dup https://github.com/microsoft/microsoft-ui-xaml/issues/4971. Thanks!

ghost commented 3 years ago

Hi! We've identified this issue as a duplicate of one that exists on somebody else's Issue Tracker. Please make sure you subscribe to the referenced external issue for future updates. Thanks for your report!

DavidZidar commented 3 years ago

@zadjii-msft Hi again, this was finally supposed to get fixed with #10508 and #11240 in v1.12.2922.0 but I'm trying it in v1.12.2931.0 and it is not fixed. Tabs still remain compacted when closing using only the keyboard and I still have to mouse over the tab bar to make them snap back to the expected size.

zadjii-msft commented 3 years ago

What the heck, you're right. This was tracked in https://github.com/microsoft/microsoft-ui-xaml/issues/4971, which was supposed to be fixed in https://github.com/microsoft/microsoft-ui-xaml/pull/4980. Maybe the fix was literally only for the built in keyboard shortcut for closing tabs, ctrl+f4, which I don't think we even use at all.

StephenLPeters commented 3 years ago

Closing a tab via the close button is not suppose to resize the tabs until you move your cursor outside of the tab strip. This is so you can close many tabs by quickly pressing your mouse button as the tabs animate under your cursor. The bug here is that when a tab closes we assume that the pointer is over the tab strip and wait until the next pointer exited event to resize the tabs. The correct solution is to resize the tabs immediately if the pointer is not over the items.

beervoley commented 3 years ago

closed via https://github.com/microsoft/microsoft-ui-xaml/pull/6160 @zadjii-msft

zadjii-msft commented 2 years ago

@beervoley Are we sure that https://github.com/microsoft/microsoft-ui-xaml/pull/6160 fixed this? I just whipped up a Terminal build with 2.8 and this doesn't seem like it's fixed.

gh-9822-aug-2022

(the numberbox at the start of the gif shows that I've definitely got 2.8, b/c #13495 is fixed)

As I close tabs, they still don't seem to resize. Maybe it's a side effect of how we're removing the tab items from the list?

https://github.com/microsoft/terminal/blob/7976e48195a569c87e37b7ca4861dfdcb7db0d7c/src/cascadia/TerminalApp/TabManagement.cpp#L532-L534

But that seems... pretty straightforward.

StephenLPeters commented 2 years ago

IDK why I looked at this, the email cought my eye, but this is also not correct: https://github.com/microsoft/microsoft-ui-xaml/blob/9670f769069535175490ee611e8a1ddf3fffd094/dev/TabView/TabView.cpp#L780 It means that if your mouse happens to be over tab item one in the tab strip when you use keyboard to close tab 7 they wont resize. I don't think its the issue you are seeing though @zadjii-msft. @ranjeshj

beervoley commented 2 years ago

@beervoley Are we sure that microsoft/microsoft-ui-xaml#6160 fixed this? I just whipped up a Terminal build with 2.8 and this doesn't seem like it's fixed.

As I close tabs, they still don't seem to resize. Maybe it's a side effect of how we're removing the tab items from the list?

https://github.com/microsoft/terminal/blob/7976e48195a569c87e37b7ca4861dfdcb7db0d7c/src/cascadia/TerminalApp/TabManagement.cpp#L532-L534

But that seems... pretty straightforward.

yup, seeing the same as @StephenLPeters outlined in a comment above - there's still a bug that if the cursor is over tabs and a tab is removed via keyboard - it still won't resize but that doesn't look what you're seeing. image not sure why this doesn't kick in: your pointer is clearly not over tabs so tabs width update should be triggered @StephenLPeters any thoughts? going over it with debugger should point the issue right away

StephenLPeters commented 2 years ago

I didn't see anything wrong with my cursory review, but maybe has to do with xaml island stuff? maybe we don't get a pointer exited because the tab well goes to the edge of the island?

beervoley commented 2 years ago

@ranjeshj the fix for this particular problem (not when cursor is still over tabs and a tab is closed via keyboard) is to change line 782 to UpdateTabWidths() instead of UpdateTabWidths(true, false) only if the user closed the tab via keyboard. This will force TabView to use all the available width to size the tabs. I don't know why we need fillAllAvailableSpace bool in UpdateTabWidths at all. Passing false to shouldUpdateTabWidths will "achieve" the same result in case we don't need to resize tabs while user's cursor is still located over them. So to summarize: 1) In case of tab closure via keyboard call UpdateTabWidths() on line 782. 2) Keyboard close should be specially handled because if the user's cursor is located on the tabs and a tab is closed via keyboard - tabview won't resize.

zadjii-msft commented 1 year ago

Upstream: https://github.com/microsoft/microsoft-ui-xaml/issues/8358