-
Notifications
You must be signed in to change notification settings - Fork 466
Full screen events #2041
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?
Full screen events #2041
Conversation
This commit introduces full screen state management for media elements. The `CommunityToolkit.Maui.Primitives` namespace was added to several files to support this feature. A new `MediaElementScreenState` enum and `FullScreenStateChangedEventArgs` class were added to represent the full screen state and event data respectively. The `IMediaElement` and `MediaElement` interfaces were updated to include a `FullScreenState` property, a `FullScreenStateChanged` event, and a `FullScreenChanged` method. The `MediaElementPage` class now subscribes to the `FullScreenStateChanged` event and logs information when triggered. The `MauiMediaElement` classes for Android and Windows, and the `MediaManagerDelegate` class for MacOS, were updated to include a `WindowsChanged` event that triggers when the full screen state changes. The `MediaManager` classes for Android, MacOS, and Windows now subscribe to the `WindowsChanged` event and update the full screen state when triggered. The `MediaManager` class for MacOS sets the `Delegate` property of the `AVPlayerViewController` to a new instance of `MediaManagerDelegate`.
- Updated MediaElementPage.xaml to include a new event handler `FullScreenStateChanged` for better responsiveness to fullscreen changes. - Removed subscription to `FullScreenStateChanged` in MediaElementPage.xaml.cs, indicating a shift in how fullscreen changes are managed. - Improved Android fullscreen support in MauiMediaElement.android.cs by invoking `OnWindowsChanged` with `FullScreenStateChangedEventArgs` to accurately track fullscreen state changes.
Updated the comment for the `OnWindowsChanged` static event in the `MediaManager` class to more accurately describe its purpose related to changes in the full screen state of the media element. The previous, less descriptive comment was replaced with a clearer explanation.
…iOld into FullScreenEvents
This commit overhauls the handling of full-screen state changes across Android, Windows, and macOS within a Maui application. A new record, `FullScreenEvents`, has been introduced in the `MediaManager` class to centralize event handling, replacing platform-specific `WindowsChanged` events with a unified approach. This refactor includes the removal of redundant `using` directives, adjustments in event invocation to utilize the new centralized event, and general code cleanup for better readability and maintainability. Platform-specific code has been updated to align with this new mechanism, streamlining the full-screen state change process across different operating systems.
…iOld into FullScreenEvents
- Removed `OnWindowsChanged` from `MediaManager` in platform-specific files (Android, macOS, Windows) due to redundancy and centralized this logic by adding a new protected method `OnWindowsChanged` to the `FullScreenEvents` record in `MediaManager.shared.cs`. This change aims to reduce code duplication and improve the management of full-screen state changes across different platforms.
Centralized the subscription to FullScreenEvents.WindowsChanged in the MediaManager class to improve maintainability and readability. Removed redundant event unsubscriptions in platform-specific MediaManager files, streamlining event management.
- Renamed `OnWindowsChanged` to `OnFullScreenStatusChanged` in `MediaManager` and `FullScreenEvents` for clarity. - Added `MediaElement` property to `FullScreenEvents` for better media element access.
In `MauiMediaElement.android.cs`, the visibility of `CurrentPlatformContext` has been changed from `readonly` to `public readonly`, enhancing its accessibility for external use. Additionally, in `MediaManager.shared.cs`, a comment has been added to acknowledge similarities with PR CommunityToolkit#1918 on the CommunityToolkit/Maui repository.
- Changed `CurrentPlatformContext` in `MauiMediaElement.android.cs` from `public` to `internal`, limiting its accessibility to within its assembly. - Modified `FullScreenEvents` in `MediaManager.shared.cs`: 1. Access modifier updated from `public` to `internal`. 2. Converted from a `record` to a `readonly record struct`, making it a value type and immutable.
|
Hi @ne0rrmatrix , do you have any idea if this still need something to be in MedieElement ? |
I am unable to move forward at this time. I do not understand what I should use for a name as @VladislavAntonyuk requested. I hope someone can suggest something. So far I have heard nothing from anyone. |
|
@bijington ty for helping me figure out what I missed. I have updated the PR to address the naming issue. |
src/CommunityToolkit.Maui.MediaElement/Primitives/FullScreenStateChangedEventArgs.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Primitives/FullScreenEvents.cs
Outdated
Show resolved
Hide resolved
|
I have removed public static event handling and it now is much more compliant with our standards. |
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
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
| public enum MediaElementScreenState | ||
| { | ||
| /// <summary> | ||
| /// Full screen. | ||
| /// </summary> | ||
| FullScreen, | ||
|
|
||
| /// <summary> | ||
| /// The default state. | ||
| /// </summary> | ||
| Default, | ||
| } |
Copilot
AI
Nov 20, 2025
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.
According to the coding guidelines, enums should have values assigned (0, 1, 2, 3) if not marked with a Flags attribute. Additionally, the enum ordering doesn't follow the guideline: for return types that may have an unknown value, use Unknown at index 0; for option types that can use the system default, use Default at index 0.
Since this represents a screen state where Default is the standard (non-fullscreen) state, it should be at index 0. Suggested fix:
public enum MediaElementScreenState
{
/// <summary>
/// The default state.
/// </summary>
Default = 0,
/// <summary>
/// Full screen.
/// </summary>
FullScreen = 1,
}| /// <param name="previousState"></param> | ||
| /// <param name="newState"></param> |
Copilot
AI
Nov 20, 2025
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.
Missing documentation for the previousState and newState parameters. The parameter documentation should describe what these parameters represent. For example:
/// <param name="previousState">The previous full screen state.</param>
/// <param name="newState">The new full screen state.</param>| /// <param name="previousState"></param> | |
| /// <param name="newState"></param> | |
| /// <param name="previousState">The previous full screen state.</param> | |
| /// <param name="newState">The new full screen state.</param> |
| /// <summary> | ||
| /// Triggers a <see cref="FullScreenState"/> change. | ||
| /// </summary> | ||
| /// <param name="newState"></param> |
Copilot
AI
Nov 20, 2025
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.
Missing documentation for the newState parameter. It should describe what the parameter represents. For example:
/// <param name="newState">The new full screen state to update to.</param>| /// <param name="newState"></param> | |
| /// <param name="newState">The new full screen state to update to.</param> |
| /// <param name="newState"></param> | ||
| public sealed class FullScreenStateChangedEventArgs(MediaElementScreenState previousState, MediaElementScreenState newState) : EventArgs | ||
| { | ||
|
|
Copilot
AI
Nov 20, 2025
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.
[nitpick] There's an extra blank line here. According to coding style best practices, there should be consistent spacing. This blank line should be removed.
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
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
| public override void WillBeginFullScreenPresentation(AVPlayerViewController playerViewController, IUIViewControllerTransitionCoordinator coordinator) | ||
| { | ||
| mediaManager.UpdateFullScreenState(MediaElementScreenState.FullScreen); | ||
| } |
Copilot
AI
Nov 24, 2025
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.
[nitpick] Missing blank line between methods. For consistency and readability, consider adding a blank line between the WillBeginFullScreenPresentation and WillEndFullScreenPresentation methods.
| } | |
| } |
| /// <param name="context">The application's <see cref="Context"/>.</param> | ||
| /// <param name="playerView">The <see cref="PlayerView"/> that acts as the platform media player.</param> | ||
| public MauiMediaElement(Context context, PlayerView playerView) : base(context) | ||
| /// <param name="mediaManager">The <see cref="mediaManager"/> that is managing the <see cref="MediaElement"/> instance.</param> |
Copilot
AI
Nov 24, 2025
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 XML documentation has an incorrect <see cref> reference. It should reference MediaManager (the type) rather than just mediaManager (the parameter name).
Suggested fix:
/// <param name="mediaManager">The <see cref="MediaManager"/> that is managing the <see cref="MediaElement"/> instance.</param>| /// <param name="mediaManager">The <see cref="mediaManager"/> that is managing the <see cref="MediaElement"/> instance.</param> | |
| /// <param name="mediaManager">The <see cref="MediaManager"/> that is managing the <see cref="MediaElement"/> instance.</param> |
Add Support for Full Screen Events for Windows, Android, iOS and Mac Catalyst.
Description of Change
Add event handlers to notify developer when full screen status changes.
API:
XAML:
Code Behind:
Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information
This is a PR to implement a solution for adding support for developers to have an the choice to see when user goes into full full screen and back again.
Note for Maintainers: If you see a need for a name change please go ahead and edit. Names are important. I have no opinion on naming of classes/functions/variables. If you see a need for them to change feel free to make those edits.