packages icon indicating copy to clipboard operation
packages copied to clipboard

[video_player] Add html 5 video poster support (thumbnail) as a VideoPlayerWebOptions

Open Ortes opened this issue 8 months ago • 5 comments

This PR adds support for the poster attribute on web by introducing a poster field in VideoPlayerWebOptions. It allows setting a thumbnail image for videos using the native HTML5 poster property, improving the out-of-the-box web experience.

Solve this issue: https://github.com/flutter/flutter/issues/166232

Pre-Review Checklist

  • [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x] I read the Tree Hygiene page, which explains my responsibilities.
  • [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • [x] I signed the CLA.
  • [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • [x] I linked to at least one issue that this PR fixes in the description above.
  • [x] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under[^1].
  • [x] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under[^1].
  • [x] I updated/added any relevant documentation (doc comments with ///).
  • [x] I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under[^1].
  • [x] All existing and new tests are passing.

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

[^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

Ortes avatar Mar 31 '25 01:03 Ortes

Where is the combination PR for initial review that this was split from (per the documented process)?

stuartmorgan-g avatar Apr 01 '25 19:04 stuartmorgan-g

Where is the combination PR for initial review that this was split from (per the documented process)?

Is this ok like this ? here is the PR for platform_interface

Ortes avatar Apr 01 '25 20:04 Ortes

From triage: @tarrinneal This should be ready for initial review.

stuartmorgan-g avatar Apr 22 '25 19:04 stuartmorgan-g

~From triage: @Ortes apologies, we forgot to follow up with next steps here. Now that this has sign-off, the next step is to split out the platform interface component into a separate PR so that we can land it, as described at https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changing-federated-plugins~

Never mind, I forgot https://github.com/flutter/packages/pull/8979 was already split out.

stuartmorgan-g avatar Jun 24 '25 18:06 stuartmorgan-g

Sorry for the mess, I messed up while trying to rebase, it is clean now

Ortes avatar Jun 25 '25 08:06 Ortes

It looks like this never got a test at the web implementation package level, which was missed during review. https://github.com/flutter/packages/blob/ca9b946ca4e36da5ab5dbcf3d110571ceb671f48/packages/video_player/video_player_web/example/integration_test/video_player_test.dart#L412 is an example of a test similar to what we should have for this.

stuartmorgan-g avatar Jun 30 '25 14:06 stuartmorgan-g

It looks like this never got a test at the web implementation package level, which was missed during review.

https://github.com/flutter/packages/blob/ca9b946ca4e36da5ab5dbcf3d110571ceb671f48/packages/video_player/video_player_web/example/integration_test/video_player_test.dart#L412

is an example of a test similar to what we should have for this.

I added tests, they are AI generated but looks good to me and they pass of course. I hope that will do it

Ortes avatar Jul 05 '25 14:07 Ortes