Repository navigation
[Fix] - [ToggleDockingManager] Honour IToolbox.IsOpen set before load - #631
Merged
sharpSteff merged 3 commits intoAug 8, 2026
Merged
sharpSteff merged 3 commits into
sharpSteff merged 3 commits into
Conversation
The manager learns about a toolbox when it registers it, which happens once it is loaded. A view model built by a dependency injection container is constructed long before that, so an IsOpen it set was decided while nobody was listening: the manager only applied IsOpenByDefault, and the value was dropped. Because a property that does not change raises no notification, the toolbox was then stuck - it already held true, so asking for true again did nothing and it could not be opened from the view model at all. The same drift arose without any startup involvement. A zone shows one toolbox at a time, so opening one collapses its sibling, and that collapse never wrote back onto the sibling's toolbox. Two toolboxes with IsOpenByDefault in one zone reproduced the stuck state on the first frame. - Replace OpenDefaultToolboxes with ApplyInitialToolboxState, which reads IsOpen as well as IsOpenByDefault and leaves what already shows alone, so running it again never collapses an open toolbox. - Write the state back when a toolbox is collapsed to make room for a sibling, and when a detached window is brought forward instead of docked. - Reconcile the toolboxes against the layout after a stored layout replaced the tree. - Fold the view model driven open path into ToggleAnchorable rather than repeating it. The duplicate had drifted: it missed the detached window case, so setting IsOpen = true on a detached toolbox tried to dock it. - Re-apply the state after a theme change, which rebuilds the bars and collapsed every open toolbox on the way. IsOpenByDefault now seeds IsOpen, which from then on is the single answer to whether a toolbox is showing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MHA6cYCRZTgYXmgycpuUdL
The style loaded from generic.xaml was held in a process wide static and handed to every manager. A Style is a DispatcherObject and keeps the affinity of the thread that created it until it is sealed, which WPF only does once it is first applied - so the first manager on a second UI thread threw "The calling thread cannot access this object because a different thread owns it" out of Style.CheckTargetType, during the layout pass, where nothing catches it and the process goes down with it. This needs a manager that actually renders a docked anchorable, which is what the new toggle toolbox state tests are the first to do: the fixtures before them build a manager on a worker thread but never lay one out, so the style was seeded on one thread and only ever read back on it. Caching per thread keeps what the field is for - parsing the pack URI once per UI thread instead of once per manager - without sharing an object that cannot be shared. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MHA6cYCRZTgYXmgycpuUdL
…the deferred show A floating window reaches the screen through a deferred dispatcher operation, so its model can be detached from the layout tree by the time the operation runs - the window hosting the docking manager was closed in the meantime, for instance. OnInitialized dereferenced _model.Root.Manager unguarded and threw a NullReferenceException out of Window.ShowHelper, where it surfaces as a failure in whatever code happened to pump the dispatcher queue. The class already expects a model without a root in OnRootUpdated and in OnClosed; this is the one place that did not. Guarding the manager lookup leaves a detached model producing an empty window instead, which is what it should produce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MHA6cYCRZTgYXmgycpuUdL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The manager learns about a toolbox when it registers it, which happens once it is loaded. A view model built by a dependency injection container is constructed long before that, so an IsOpen it set was decided while nobody was listening: the manager only applied IsOpenByDefault, and the value was dropped. Because a property that does not change raises no notification, the toolbox was then stuck - it already held true, so asking for true again did nothing and it could not be opened from the view model at all.
The same drift arose without any startup involvement. A zone shows one toolbox at a time, so opening one collapses its sibling, and that collapse never wrote back onto the sibling's toolbox. Two toolboxes with IsOpenByDefault in one zone reproduced the stuck state on the first frame.
IsOpenByDefault now seeds IsOpen, which from then on is the single answer to whether a toolbox is showing