fix(tabs): stop a tell from a popped-out partner hijacking the main window
Reported from the field: a new tell from someone whose tab is popped out throws the main window onto the General tab, every time. The router revealed the tab by activating it in the main window. That window does not display a popped-out tab -- PickMainActiveTab re-anchors on the next frame, and it anchors to the FIRST non-popped tab in list order, not to the one the user was reading. So the reveal did nothing it intended and threw away the active tab on the way. Nothing needed revealing in the first place: the tab was already on screen in its own window. That decision now lives in PlanTellReveal, next to the pop-out helpers it belongs with, pure and pinned by nine facts -- the mode, the switch and the popped-out state have eight combinations between them and only two of them should touch anything. ActivateTab refuses a popped-out tab outright as well. The router is the caller that got it wrong, but the invariant belongs to the window: its active tab is never one that something else is drawing.
This commit is contained in:
@@ -75,17 +75,23 @@ internal sealed class TellRouterService : IDisposable
|
||||
if (tab == null)
|
||||
return; // nothing to reveal (auto-tell-tabs off -> no tab created)
|
||||
|
||||
switch (mode)
|
||||
{
|
||||
case TellAutoOpenMode.Sidebar:
|
||||
case TellAutoOpenMode.TopTab:
|
||||
// Switching to the tab on every tell is user-gated
|
||||
// (TellAutoOpenSwitchAlways, default on); when off the tab still
|
||||
// appears with its unread badge but the active tab is left alone.
|
||||
// The mode also picks the layout, so Sidebar vs TopTab are actually
|
||||
// distinct outcomes, not the same ActivateTab.
|
||||
if (Plugin.Config.TellAutoOpenSwitchAlways)
|
||||
// appears with its unread badge but the active tab is left alone. A
|
||||
// tab that is already popped out needs no reveal at all -- it is on
|
||||
// screen, and pulling the main window onto it costs the user the tab
|
||||
// they were reading.
|
||||
var reveal = TabLifecycleHelpers.PlanTellReveal(
|
||||
mode,
|
||||
Plugin.Config.TellAutoOpenSwitchAlways,
|
||||
Plugin.Instance.ChannelPopoutPool.IsOpen(tab.Identifier)
|
||||
);
|
||||
|
||||
switch (reveal)
|
||||
{
|
||||
case TabLifecycleHelpers.TellReveal.MainWindow:
|
||||
// The mode also picks the layout, so Sidebar vs TopTab are
|
||||
// actually distinct outcomes, not the same ActivateTab.
|
||||
var wantLayout =
|
||||
mode == TellAutoOpenMode.TopTab
|
||||
? MainWindowLayoutMode.TopTabs
|
||||
@@ -97,13 +103,9 @@ internal sealed class TellRouterService : IDisposable
|
||||
}
|
||||
|
||||
Plugin.Instance.MainWindow?.ActivateTab(tab);
|
||||
}
|
||||
|
||||
break;
|
||||
case TellAutoOpenMode.Popout:
|
||||
// IsOpen-guard: don't double-pop a tab the AutoTellTabsOpenAsPopout
|
||||
// path already opened (the two switches stay decoupled).
|
||||
if (!Plugin.Instance.ChannelPopoutPool.IsOpen(tab.Identifier))
|
||||
|
||||
case TabLifecycleHelpers.TellReveal.Popout:
|
||||
Plugin.Instance.ChannelPopoutPool.TryOpen(tab);
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -162,6 +162,14 @@ internal sealed class MainWindow : Window, IFocusableChatWindow
|
||||
// header pick strips tell-state and resets unread the way a real click does.
|
||||
internal void ActivateTab(Tab tab)
|
||||
{
|
||||
// A popped-out tab is not a surface this window owns. Taking it as
|
||||
// active does not show it -- PickMainActiveTab re-anchors on the next
|
||||
// frame, and it anchors to the first non-popped tab, which is not the
|
||||
// one the user was reading. Callers that mean "bring it forward" have
|
||||
// to reach for the pool instead.
|
||||
if (_pool.IsOpen(tab.Identifier))
|
||||
return;
|
||||
|
||||
if (ReferenceEquals(_activeTab, tab))
|
||||
return;
|
||||
|
||||
|
||||
@@ -180,6 +180,39 @@ internal static class TabLifecycleHelpers
|
||||
return null;
|
||||
}
|
||||
|
||||
// What an incoming tell should do about the tab it belongs to.
|
||||
internal enum TellReveal
|
||||
{
|
||||
None,
|
||||
MainWindow,
|
||||
Popout,
|
||||
}
|
||||
|
||||
// POP-1f: the alreadyPopped case is the whole reason this is a function.
|
||||
//
|
||||
// Revealing a popped-out tab in the main window looks like it does nothing,
|
||||
// and then does something worse: PickMainActiveTab re-anchors on the next
|
||||
// frame, and it anchors to the FIRST non-popped tab, not to the one the user
|
||||
// was reading. So a tell from a partner whose tab is popped out threw the
|
||||
// main window back to the first tab every single time.
|
||||
//
|
||||
// The tab is already on screen in its own window. There is nothing to
|
||||
// reveal.
|
||||
//
|
||||
// Pure + Dalamud-free.
|
||||
// TEST-MIRROR: ../../../Hellion Build test/_Helpers/PlanTellRevealTests.cs
|
||||
internal static TellReveal PlanTellReveal(
|
||||
TellAutoOpenMode mode,
|
||||
bool switchAlways,
|
||||
bool alreadyPopped
|
||||
) =>
|
||||
mode switch
|
||||
{
|
||||
TellAutoOpenMode.Off => TellReveal.None,
|
||||
TellAutoOpenMode.Popout => alreadyPopped ? TellReveal.None : TellReveal.Popout,
|
||||
_ => switchAlways && !alreadyPopped ? TellReveal.MainWindow : TellReveal.None,
|
||||
};
|
||||
|
||||
// POP-1e: popout-aware sibling of WrapTabIndex for the ChatTabForward/Backward
|
||||
// keybind. Steps from current by delta's sign (±1), wrapping, and returns the
|
||||
// first index whose tab is NOT popped out within tabs.Count steps; returns current
|
||||
|
||||
Reference in New Issue
Block a user