fix(lifecycle): Ensure layout realization states remain monotonic#48
fix(lifecycle): Ensure layout realization states remain monotonic#48google-labs-jules[bot] wants to merge 1 commit into
Conversation
The layout realization properties (`ShouldLoadActiveConversationRoot` and `ShouldLoadTranscriptSurface`) were incorrectly mirroring their corresponding `ShouldShow` properties. This caused the visual tree to unload when a temporary blocking overlay appeared (e.g., during rapid session switching or history hydration), losing UI state such as scroll position and wasting CPU cycles upon recreation. This commit updates `ChatConversationSurfaceProjectionCoordinator` to track the historical loaded state. Once the load state evaluates to `true`, it latches and remains `true` (monotonic), while the `ShouldShow` property continues to correctly manage visibility without destroying the underlying visual tree. Tests in `ChatConversationSurfaceProjectionCoordinatorTests` and `ChatViewModelTests` have been updated to reflect and verify this new monotonic 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
Observed Problem:
Layout realization states (the properties typically bound to
x:Loadin the UI, such asShouldLoadActiveConversationRootandShouldLoadTranscriptSurface) were regressing tofalsewhen temporary blocking overlays (e.g., during history hydration or rapid session switching) became visible. This caused the visual tree to be destroyed and re-created, leading to lost state (like scroll position) and wasted CPU cycles.Root Cause:
In
ChatConversationSurfaceProjectionCoordinator.cs, theShouldLoad...properties were directly mirroring theirShouldShow...counterparts. SinceShouldShow...correctly becomesfalsewhen a blocking mask is active, theShouldLoad...properties also becamefalse, effectively un-loading the UI elements instead of just hiding them.Why the Fix is Safe Architecturally:
The fix is tightly encapsulated within the
ChatConversationSurfaceProjectionCoordinator. By introducing boolean latches (_hasLoadedActiveConversationRootand_hasLoadedTranscriptSurface), the coordinator now ensures that once these elements are instructed to load, theirShouldLoadstatus remains monotonicallytrue. This perfectly decouples layout realization (x:Load) from visual state visibility (Visibility), aligning with Uno/WinUI lifecycle best practices. No native controls are overridden, and no business rules have been leaked into the View or code-behind.Verification Commands Run:
dotnet build src/SalmonEgg.Presentation.Core/SalmonEgg.Presentation.Core.csproj --configuration Debug dotnet test tests/SalmonEgg.Presentation.Core.Tests/SalmonEgg.Presentation.Core.Tests.csproj --configuration Debug --no-restoreRemaining Unverified GUI-Only Risks:
Since the fix alters the
x:Loadlifecycle purely via ViewModel projections, the actual memory retention and visual performance impact when switching heavily populated chats relies on Uno Platform's underlyingx:Loadbehavior. While structurally safe, specific platform rendering nuances during repeated hide/show cycles are not fully covered by the core unit tests.PR created automatically by Jules for task 3378912116509574605 started by @YoungSx