Skip to content

Conversation

@ne0rrmatrix
Copy link
Member

@ne0rrmatrix ne0rrmatrix commented Nov 9, 2024

  • Feature/Proposal

Description of Change

Update MediaElement Transport Controls for Windows. Adds Support for Full Screen button in the controls. Controls are updated to have play, pause, Fast Forward, Next, Previous, Inline Volume Controls, Repeat button, Cast Button and an Aspect button.

Linked Issues

PR Checklist

Additional information

This updates the windows controls to support all the Media Buttons you could want.

image

image

Introduced a new embedded resource, `ResourceDictionary.windows.xaml`, in the `CommunityToolkit.Maui.MediaElement.csproj` project file.

Updated `MauiMediaElement.windows.cs`:
- Imported new namespaces.
- Added `CustomTransportControls` field and methods for loading resource dictionary and applying custom styles.
- Updated constructor to load resource dictionary and set custom transport controls.
- Added `LoadResourceDictionary`, `ApplyCustomStyle`, and `SetTransportControls` methods.
- Updated `Dispose` method to handle custom transport controls.
- Removed `OnMediaPlayerElementPointerMoved` method and related code.
- Updated `OnFullScreenButtonClick` method to remove fullscreen button references.

Updated `MediaManager.windows.cs`:
- Disabled `SystemMediaTransportControls` in `CreatePlatformView`.

Added `CustomTransportControls` class in `CommunityToolkit.Maui.Primitives`:
- Imported necessary namespaces.
- Defined `FullScreenButton` property.
- Implemented `OnTemplateLoaded` event.
- Overrode `OnApplyTemplate` method to add full-screen button.
- Added `FullScreenButton_Click` event handler.
- Introduced `isFullScreen` field to track full-screen state.
Moved RightSeparator before PlaybackRateButton in ResourceDictionary.windows.xaml. Added RightSeparator with Height='0', Width='0', and Margin='0,0'. Reordered properties of PlaybackRateButton without changing values. Removed original RightSeparator after PlaybackRateButton.
Re-added various properties and elements to styles and templates in `ResourceDictionary.windows.xaml` to ensure consistent UI appearance and behavior. Added new buttons like `ZoomButton` and `CastButton` to the media controls command bar. Enhanced `customTransportcontrols` and `CommandBarStyle` with additional properties.
Removed the `UseLayoutRounding` property and adjusted the `Margin`
property from `0,0,0,20` to `0,0,0,10`. Added `VerticalContentAlignment`
set to `Center` and `VerticalAlignment` set to `Bottom`. Introduced
`CornerRadius` of `8,8,8,8` to the `Grid` element in the
`ResourceDictionary.windows.xaml` file.
- Cleaned up `using` directives by removing redundant `using CommunityToolkit.Maui.Core;` and reordering the remaining directives.
- Changed `CustomTransportControls` class from public sealed to sealed partial.
- Removed XML documentation comments for `CustomTransportControls` class and its members.
- Changed `FullScreenButton` property from nullable `AppBarButton` with a private setter to non-nullable `AppBarButton` initialized with a new instance.
- Removed null check for `FullScreenButton` in `FullScreenButton_Click` method as `FullScreenButton` is no longer nullable.
@ne0rrmatrix ne0rrmatrix added the 📽️ MediaElement Issue/PR that has to do with MediaElement label Nov 9, 2024
@ne0rrmatrix ne0rrmatrix marked this pull request as ready for review November 11, 2024 23:20
@ne0rrmatrix ne0rrmatrix added the needs discussion Discuss it on the next Monthly standup label Dec 5, 2024
Copy link
Contributor

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.

Copilot reviewed 4 out of 5 changed files in this pull request and generated no suggestions.

Files not reviewed (1)
  • src/CommunityToolkit.Maui.MediaElement/CommunityToolkit.Maui.MediaElement.csproj: Language not supported
Comments skipped due to low confidence (3)

src/CommunityToolkit.Maui.MediaElement/Primitives/CustomTransportControls.windows.cs:8

  • The variable name FullScreenButton should be renamed to fullScreenButton to follow the naming conventions.
public AppBarButton FullScreenButton = new();

src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.windows.cs:53

  • The LoadResourceDictionary method does not handle the case where the resource dictionary might not load correctly, which could lead to a runtime error. Add a null check for resourceDictionary.
var resourceDictionary = (Microsoft.UI.Xaml.ResourceDictionary)XamlReader.Load(xaml);

src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.windows.cs:167

  • The OnFullScreenButtonClick method does not check if CurrentPage is null before using it, which could lead to a NullReferenceException. Add a null check for CurrentPage.
var currentPage = CurrentPage;

@TheCodeTraveler TheCodeTraveler removed the needs discussion Discuss it on the next Monthly standup label Jan 16, 2025
@vhugogarcia vhugogarcia self-requested a review January 22, 2025 17:50
Copy link
Contributor

@vhugogarcia vhugogarcia left a comment

Choose a reason for hiding this comment

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

Hey @ne0rrmatrix, thanks so much for this PR! 🙌 I downloaded and tested the changes locally, and they worked perfectly. I really appreciate you fixing that crash issue when the volume was set to 0 on the transport controls for Windows—such a relief to have that sorted.

I tested everything on Windows 11, and it’s all running smoothly. Great work!

@brminnick how can I merge this PR into the main branch? I cannot see the option to do it hehe. 😃

@TheCodeTraveler TheCodeTraveler enabled auto-merge (squash) January 22, 2025 19:18
@TheCodeTraveler TheCodeTraveler merged commit dec248e into CommunityToolkit:main Jan 22, 2025
15 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

📽️ MediaElement Issue/PR that has to do with MediaElement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants