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

Reordering tabs messes up relation between tabs and panes #15121

Closed
Tracked by #14957
djdv opened this issue Apr 5, 2023 · 3 comments · Fixed by #15178 or #15313
Closed
Tracked by #14957

Reordering tabs messes up relation between tabs and panes #15121

djdv opened this issue Apr 5, 2023 · 3 comments · Fixed by #15178 or #15313
Assignees
Labels
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. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree.

Comments

@djdv
Copy link

djdv commented Apr 5, 2023

Windows Terminal version

6244896

Windows build number

10.0.25330.0

Other Software

No response

Steps to reproduce

Open multiple tabs. Reorder them.

Expected Behavior

No response

Actual Behavior

The relation/index between tabs and panes is now broken.
Selecting a tab will select a pane which does not belong to that tab. Closing panes will close the wrong tab.
See: https://www.youtube.com/watch?v=Qa9jxFnvI8U

@djdv djdv 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 Apr 5, 2023
@zadjii-msft zadjii-msft added Severity-Blocking We won't ship a release like this! No-siree. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 11, 2023
@zadjii-msft zadjii-msft added this to the Terminal v1.18 milestone Apr 11, 2023
@zadjii-msft zadjii-msft self-assigned this Apr 11, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Apr 11, 2023
@lhecker lhecker added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. Priority-1 A description (P1) labels Apr 12, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 12, 2023
@zadjii-msft
Copy link
Member

zadjii-msft commented Apr 13, 2023

More concrete repro steps:

  • Open two windows
  • In one window, open two tabs. Pwsh and CMD are good, so they're clearly separate.
  • Reorder those two.
  • Drag the inactive one to the other window.
  • Experience having the active term core in both windows!

It's so obvious:

Wait but no, _onTabStripDrop doesn't ever cause a _MoveTab. It triggers a _RequestReceiveContentHandlers, which is bubbled to the sender, which does a SendContentToOther, which does a _MoveContent on the dragged tab. Hmm.

@zadjii-msft
Copy link
Member

zadjii-msft commented Apr 13, 2023

oh its so bad

Just reordering tabs, no tearout, just breaks the entire relationship of it. I wonder if adding these drag/drop handlers somehow broke the reorder events we get...

It sure did. We don't get TabView.TabItemsChanged anymore now that we get TabDragStarting & TabStripDrop. WELP. Let's see what we can do.

@zadjii-msft
Copy link
Member

Upstream: microsoft/microsoft-ui-xaml#8388

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Apr 13, 2023
microsoft-github-policy-service bot pushed a commit that referenced this issue Apr 17, 2023
…15178)

TL;DR: we stopped getting `TabView.TabItemsChanged`. This meant that the
tab view would change its apparent order, but we wouldn't change the
backing tab order.

I'm fixing this by grabbing the index of the tab that starts the drag,
and the index of the tab view item at the end of the drag, and using
that to reorder our backing list.

Closes #15121

Upstream microsoft/microsoft-ui-xaml#8388

Regressed in #15078 - I'm pretty confident about this, since I've got a
1.18.931 build of the Terminal with tear-out, but not MUX 2.8.
DHowett pushed a commit that referenced this issue May 10, 2023
Reverts #15164, because that's fixed upstream now.

Closes #15139. 

Reverts #15178, but also closes #15121, because that's fixed upstream.

see also:
* microsoft/microsoft-ui-xaml#8430
* microsoft/microsoft-ui-xaml#8420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
3 participants