Skip to content

Conversation

@HotCakeX
Copy link
Contributor

The carousel on the home page visually leaks to other pages after navigation, that means we can see the carousel on other pages briefly after clicking on the tiles in the home page carousel. This PR fixes this problem completely; however, you need to accept my previous PR first, otherwise this change won't work, and you will get an error after navigation: https://github.com/microsoft/ai-dev-gallery/pull/458/files

Please see the videos below for more details as they exactly show the issue this PR fixes:


Before

BEFORE.mp4

After

AFTER.mp4

HotCakeX and others added 6 commits September 28, 2025 21:06
Event handlers in the HoverLight weren't being unsubscribed On Disconnected.
OpacityMaskView wasn't handling resources properly on unload, causing memory leaks and also changing 

```csharp
private static CompositionBrush GetVisualBrush(UIElement element)
```

to 

```csharp
   private static CompositionSurfaceBrush GetVisualBrush(UIElement element)
```

improves performance.
The carousel on the home page visually leaks to other pages after navigation, that means we can see the carousel on other pages briefly after clicking on the tiles in the home page carousel. This PR fixes this problem completely, however, you need to accept my previous PR first, otherwise this change won't work: https://github.com/microsoft/ai-dev-gallery/pull/458/files
@weiyuanyue
Copy link
Contributor

Appreciate the intent to ensure resources are released when navigating away. However, I don’t think we should add this change as-is:

  • WinUI navigation already triggers Unloaded and tears down the visual tree. Forcibly clearing rootGrid.Children and setting this.Content = null can interfere with back navigation, page caching, and transition animations, and may cause blank pages or NREs on return.
  • This assumes this.Content is a Grid, which isn’t guaranteed long-term and is fragile.
  • Clearing mostRecentlyUsedItems is unnecessary; the page-level collection is repopulated on Loaded and will be GC’d when the page is disposed. If we need a fresh list per navigation, we can reset it in OnNavigatedTo/Loaded without tearing down the UI

@HotCakeX
Copy link
Contributor Author

HotCakeX commented Oct 16, 2025

Hi @weiyuanyue
Thanks for reviewing my PR, I believe I've addressed all of the points you raised while fixing the visual bug shown in the video. Will you please do another review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants