Skip to content

Conversation

ne0rrmatrix
Copy link
Contributor

  • Bug fix

Description of Change

Support for Media Element on a FlyoutPage does not currently work on IOS. This adds support for it on IOS and Mac Catalyst.

Linked Issues

PR Checklist

Additional information

Windows and android already support playback on a FlyoutPage.

@TheCodeTraveler
Copy link
Collaborator

@ne0rrmatrix How are you able to test + verify this fix works for a FlyoutPage?

This PR doesn't include an update to the sample app or additional unit tests. Any time we fix a bug, we should add a unit test, if possible, and add update the sample app to demonstrate it working (aka a Regression Test).

@ne0rrmatrix
Copy link
Contributor Author

I will update the PR with a sample and create tests. Ty.

@ne0rrmatrix
Copy link
Contributor Author

I tested against the provided sample showing isdue

Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend doing two things to make this more robust:

  1. Update PageExtensions.GetCurrentPage() to match the most-recent implementation of Microsoft.Maui.Controls.Platform.PageExtensions
  2. Update TryGetCurrentPage to use the GetCurrentPage() extension method which already checks for FlyoutPage:
static bool TryGetCurrentPage([NotNullWhen(true)] out Page? currentPage)
{
	currentPage = null;

	if (Application.Current?.Windows is null)
	{
		return false;
	}

	if (Application.Current.Windows.Count is 0)
	{
		throw new InvalidOperationException("Unable to find active Window");
	}

	if (Application.Current.Windows.Count > 1)
	{
		// We are unable to determine which Window contains the ItemsView that contains the MediaElement when multiple ItemsView are being used in the same page
		// TODO: Add support for MediaElement in an ItemsView in a multi-window application
		throw new NotSupportedException("MediaElement is not currently supported in multi-window applications");
	}

	var window = Application.Current.Windows[0];

	// If using Shell, return the current page
	if (window.Page is Shell { CurrentPage: not null } shell)
	{
		currentPage = shell.CurrentPage;
		return true;
	}

	// If not using Shell, use the ModelNavigationStack to check for any pages displayed modally
	if (TryGetModalPage(window, out var modalPage))
	{
		currentPage = modalPage;
		return true;
	}

	if (window.Page is Page page)
	{
		currentPage = page.GetCurrentPage();
		return true;
	}

	return false;

	static bool TryGetModalPage(Window window, [NotNullWhen(true)] out Page? page)
	{
		page = window.Navigation.ModalStack.LastOrDefault();
		return page is not null;
	}
}

Update TryGetCurrentPage to use GetCurrentPage
@ne0rrmatrix
Copy link
Contributor Author

I have updated page extensions and tryGetCurrentPage as requested. I will start trying to figure out how to implement a FlyoutPage with sample app. Implementing the logic to handle a flyout page and a non shell app will be challenging.

Device specific tests for IOS/Mac to test this can be done. I am not sure if that is what you are looking for? If so I can do that. I hope I am not too off track with that line of reasoning.

@dotnet-policy-service dotnet-policy-service bot added stale The author has not responded in over 30 days help wanted This proposal has been approved and is ready to be implemented labels Jun 7, 2025
Removed dependency on CommunityToolkit.Maui.UnitTests.Mocks and changed inheritance from BaseHandlerTest to BaseTest, updating the testing context for improved clarity and functionality.
@Copilot Copilot AI review requested due to automatic review settings August 3, 2025 22:51
Copy link
Contributor

@Copilot Copilot AI left a 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 fixes a bug where MediaElement was not working on FlyoutPage in iOS and Mac Catalyst platforms. The fix centralizes page navigation logic by creating a shared PageExtensions.GetCurrentPage method that properly handles different page types including modal pages, FlyoutPages, Shell, and page containers.

Key changes:

  • Added comprehensive page navigation logic to handle FlyoutPage scenarios
  • Refactored platform-specific code to use shared extension method
  • Added unit tests to verify the page resolution functionality

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
PageExtensions.shared.cs Implements centralized page resolution logic with support for modal pages, FlyoutPage, Shell, and page containers
MauiMediaElement.macios.cs Simplifies iOS/Mac Catalyst implementation to use the shared page resolution method
PageExtensionsTests.cs Adds comprehensive unit tests covering all page resolution scenarios
Comments suppressed due to low confidence (2)

src/CommunityToolkit.Maui.MediaElement/Extensions/PageExtensions.shared.cs:29

  • Missing closing brace for the GetCurrentPage method. The method body appears to be incomplete as it's missing the closing brace.
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted This proposal has been approved and is ready to be implemented stale The author has not responded in over 30 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS MediaElement in CollectionView on FlyoutPage
2 participants