Fix layout lifecycle monotonicity during temporary overlay visibility#57
Fix layout lifecycle monotonicity during temporary overlay visibility#57google-labs-jules[bot] wants to merge 1 commit into
Conversation
This commit updates `ChatConversationSurfaceProjectionCoordinator` to ensure that once `ShouldLoadActiveConversationRoot` and `ShouldLoadTranscriptSurface` become `true`, they do not regress to `false`. This properly separates the view loading realization (`ShouldLoad...` tied to `x:Load`) from visual state visibility (`ShouldShow...`), preventing expensive UI layout tear-downs and ensuring that chat surfaces remain in memory correctly while temporary blocking overlays are shown. Updates relevant tests in `ChatConversationSurfaceProjectionCoordinatorTests` and `ChatViewModelTests` to expect monotonic layout behavior.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Closing during PR triage: this is a stale or duplicate layout-loading monotonicity attempt. #49 is the clean green candidate for this behavior; this PR is superseded, failing/conflicting, or mixed-scope and should not be merged as-is. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
1 participant
Fix layout lifecycle monotonicity during temporary overlay visibility
This commit updates
ChatConversationSurfaceProjectionCoordinatorto ensure that onceShouldLoadActiveConversationRootandShouldLoadTranscriptSurfacebecometrue, they do not regress tofalse. This properly separates the view loading realization (ShouldLoad...tied tox:Load) from visual state visibility (ShouldShow...), preventing expensive UI layout tear-downs and ensuring that chat surfaces remain in memory correctly while temporary blocking overlays are shown. Updates relevant tests inChatConversationSurfaceProjectionCoordinatorTestsandChatViewModelTeststo expect monotonic layout behavior.The observed problem: When a temporary overlay, like a connection masking view or session switch preview, is shown, the ChatViewModel drops the
ShouldLoadActiveConversationRootandShouldLoadTranscriptSurfacevalues to false. This violates thex:Loadmonotonic rule where things shouldn't regress just because a transient overlay obscures them, causing tearing down of the tree and losing things like warm scroll resume.The root cause:
ChatConversationSurfaceStatePresenterandChatConversationSurfaceProjectionCoordinatorcoupledShouldLoad...andShouldShow...directly in mapping. If a surface shouldn't be shown due to overlay visibility, its load command was also negated.Why the fix is safe architecturally: By adding state properties to track if
ShouldShowActiveConversationRootandShouldShowTranscriptSurfacehave ever been true and then using those to gate theShouldLoad...projection property, we properly separate layout realization intent from current visual visibility. This complies fully with the Uno Platform view rules without touching code behind.Verification:
dotnet build SalmonEgg.sln --configuration Debugdotnet test tests/SalmonEgg.Presentation.Core.Tests/SalmonEgg.Presentation.Core.Tests.csproj --configuration Debug --no-restoreUnverified GUI-only risks: None known. No direct XAML was touched and it correctly maintains
ShouldLoad...state behind the scenes safely.PR created automatically by Jules for task 6589520212321133970 started by @YoungSx