Skip to content
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

[video_player_web] Adjust Web implementation to the new platform interface #8528

Conversation

FirentisTFW
Copy link
Contributor

@FirentisTFW FirentisTFW commented Jan 29, 2025

This PR adjusts the Web video_player implementation to the new platform interface which was introduced in #8453. Specifically, it renames textureId to playerId (which actually makes sense here, as there are no texture IDs on Web, it was just a "simulation" to comply with the interface).

This PR does not fix any issue directly, but it's related to #86613.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@FirentisTFW FirentisTFW force-pushed the feature/video-player-web-platform-view-support branch from 711a9fe to 3e79a30 Compare January 29, 2025 17:24
@FirentisTFW FirentisTFW marked this pull request as ready for review January 29, 2025 17:43
@FirentisTFW FirentisTFW requested a review from ditman as a code owner January 29, 2025 17:43
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick cleanup!

@FirentisTFW
Copy link
Contributor Author

@ditman Would you like to do the secondary review?

@stuartmorgan
Copy link
Contributor

@FirentisTFW Now that you have the magic Member badge, you no longer need a secondary review; feel free to land it :)

(If this changed non-trivial web logic I'd definitely want someone with more web expertise than me to review it, but this is a straightforward PR.)

@FirentisTFW FirentisTFW merged commit 57e8599 into flutter:main Jan 30, 2025
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants