Fix monotonic lazy-loading lifecycle states for chat overlays#65
Fix monotonic lazy-loading lifecycle states for chat overlays#65google-labs-jules[bot] wants to merge 2 commits into
Conversation
Ensure layout loading states for chat conversation root and transcript surfaces remain monotonic to prevent x:Load elements from rematerializing when temporarily hidden by blocking overlays.
|
👋 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 trial has ended. Reactivate Greptile to resume code reviews.
Ensure layout loading states for chat conversation root and transcript surfaces remain monotonic to prevent x:Load elements from rematerializing when temporarily hidden by blocking overlays.
Observed Problem:
If a temporary blocking overlay appears,
ShouldShowActiveConversationRootandShouldShowTranscriptSurfacebecome false, which incorrectly cascades toShouldLoadActiveConversationRootandShouldLoadTranscriptSurfacebecoming false. In Uno Platform/WinUI, un-loading layout elements when they should only be hidden breaks monotonic lifecycle state, causing the layout to un-realize.Root Cause:
ChatConversationSurfaceProjectionCoordinatormapped the lazy-loading fieldsShouldLoadActiveConversationRootandShouldLoadTranscriptSurfacedirectly to their respectiveShouldShow...visibility properties without latching thetruestate once rendered.Why the fix is safe architecturally:
The fix is well-contained within the
ChatConversationSurfaceProjectionCoordinator. By using instance-level boolean flags to latch the state once they become true, it correctly aligns with XAMLx:Loadbehavior, avoiding tearing down and recreating complex visual trees.Exact verification commands run:
dotnet build SalmonEgg.sln --configuration Debugdotnet test tests/SalmonEgg.Presentation.Core.Tests/SalmonEgg.Presentation.Core.Tests.csproj --configuration Debug --no-restore(Note: Global source tests have flaky unrelated issues, but isolated Core component tests forChatViewModelpassed.)dotnet test tests/SalmonEgg.Presentation.Core.Tests/SalmonEgg.Presentation.Core.Tests.csproj --configuration Debug --no-restore --filter "FullyQualifiedName~ChatConversationSurfaceProjectionCoordinatorTests"dotnet test tests/SalmonEgg.Presentation.Core.Tests/SalmonEgg.Presentation.Core.Tests.csproj --configuration Debug --no-restore --filter "FullyQualifiedName~SalmonEgg.Presentation.Core.Tests.Chat.ChatViewModelTests.VisibleConversationContent_WhenShellHasNewLatestIntent_HidesHeaderTranscriptAndInputBeforeConversationSwitchCommits"Remaining unverified GUI-only risks:
This is a ViewModel behavior update so UI changes are deterministic based on boolean bindings, but complete End-to-End GUI tests might be required depending on visual nuances in Native rendering.
PR created automatically by Jules for task 1552983201521105879 started by @YoungSx