-
Notifications
You must be signed in to change notification settings - Fork 441
Add Support for Multiple ItemViews in MediaElement #2807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add Support for Multiple ItemViews in MediaElement #2807
Conversation
- Replace platform-specific directives with runtime checks in MediaElementMultipleWindowsPage.cs - Change LaunchMode to Multiple in MainActivity.cs - Update Info.plist to support multiple scenes - Simplify UIViewController logic in MauiMediaElement.macios.cs - Update methods to handle multi-window applications in MauiMediaElement.macios.cs - Add SceneDelegate classes for iOS and Mac Catalyst
Added a using directive for CommunityToolkit.Maui.Sample.Constants. Removed the preprocessor directive for the secondWindow variable, replacing it with a nullable Window? type to allow for more flexible handling across platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for multiple ItemsView
instances and multi-window scenarios on iOS and MacCatalyst for the MediaElement
control in .NET MAUI. Key changes include updating the internal view‐controller resolution logic, adding scene‐delegate classes and Info.plist entries for multi‐scene support, and updating the sample app to demonstrate multiple windows.
- Refactored
MauiMediaElement.macios.cs
to use handlers instead of reflection forItemsView
resolution - Added
SceneDelegate
classes and Info.plist configuration entries for iOS and MacCatalyst multi‐scene support - Updated sample pages and Android
LaunchMode
to demonstrate multiple windows
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.macios.cs | New handler-based logic for finding host view controllers; removed reflection code |
samples/CommunityToolkit.Maui.Sample/Platforms/iOS/SceneDelegate.cs | Added empty SceneDelegate class to support multiple scenes |
samples/CommunityToolkit.Maui.Sample/Platforms/iOS/Info.plist | Added UIApplicationSceneManifest for multi-window support |
samples/CommunityToolkit.Maui.Sample/Platforms/MacCatalyst/SceneDelegate.cs | Added SceneDelegate class for MacCatalyst |
samples/CommunityToolkit.Maui.Sample/Platforms/MacCatalyst/Info.plist | Added multi-scene manifest entries |
samples/CommunityToolkit.Maui.Sample/Platforms/Android/MainActivity.cs | Changed LaunchMode to Multiple for Android multi-window |
samples/CommunityToolkit.Maui.Sample/Pages/Views/MediaElement/MediaElementMultipleWindowsPage.cs | Removed preprocessor guards to always define secondWindow ; missing initialization |
Comments suppressed due to low confidence (2)
src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.macios.cs:97
- The method is named with a
Try
prefix but returns void. Consider renaming it toGetItemsViewOnPage
or changing it to return a bool to match theTry
convention.
static void TryGetItemsViewOnPage(List<Page> currentPage, out List<ItemsView> itemsView)
samples/CommunityToolkit.Maui.Sample/Pages/Views/MediaElement/MediaElementMultipleWindowsPage.cs:11
- [nitpick] Constant names should use PascalCase by convention. Consider renaming to
BuckBunnyMp4Url
andElephantsDreamMp4Url
.
const string buckBunnyMp4Url = "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4";
// We may be using Multi-window support, so we need to traverse all Windows to find the current page in a multi-window application. | ||
// We then check if the VisualElement is an ItemsView (e.g. CarouselView or CollectionView) and add it to the itemsView list/ | ||
|
||
itemsView = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The out parameter itemsView
is initialized but never populated with the discovered items. You should assign the local itemsViewsOnPage
list to itemsView
(e.g. itemsView = itemsViewsOnPage;
).
Copilot uses AI. Check for mistakes.
viewController = itemsView?.Where(item => item.Handler is not null) | ||
.Select(item => GetUIViewController(item.Handler)) | ||
.FirstOrDefault(viewController => viewController is not null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unconditionally overriding viewController
here can set it to null if no matching item is found, losing the fallback to the root view controller. Wrap this assignment in a check (e.g. only override if the query returns a non-null controller).
viewController = itemsView?.Where(item => item.Handler is not null) | |
.Select(item => GetUIViewController(item.Handler)) | |
.FirstOrDefault(viewController => viewController is not null); | |
var potentialViewController = itemsView?.Where(item => item.Handler is not null) | |
.Select(item => GetUIViewController(item.Handler)) | |
.FirstOrDefault(viewController => viewController is not null); | |
if (potentialViewController is not null) | |
{ | |
viewController = potentialViewController; | |
} |
Copilot uses AI. Check for mistakes.
#endif | ||
const string buckBunnyMp4Url = "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4"; | ||
const string elephantsDreamMp4Url = "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ElephantsDream.mp4"; | ||
readonly Window? secondWindow; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secondWindow
is declared but never initialized in the constructor, so OnAppearing
will always return early. Reintroduce its assignment using your new URL constants.
Copilot uses AI. Check for mistakes.
Sorry I just realized this does not actually work! It does not crash. But it does not provide the correct order of collection view items on page. I still have to work that out. I will continue working on this. |
@ne0rrmatrix I have an issue when using My issue is this: on MAUI I think code that searcher for iOS media player parent it's written in a way, that if on a |
I've just seen this 😅 |
Description of Change
Add Mac/iOS support for multiple Item Views when used in CollectionView or CarouselView when using MediaElement. This also adds support for Multi-Window on iOS.
ItemView
controls #2247PR Checklist
approved
(bug) orChampioned
(feature/proposal)main
at time of PRAdditional information
This will add feature parity when using CollectionView and CarouselView with iOS and MacOS to match current support in Android and Windows when using Media Element. This adds support for multiple items views and supports multiple windows. This PR is focused on MacOS and iOS only. The required support is already available on other platforms.