Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[video_player] Add optional web options #6715

Closed
wants to merge 2 commits into from

Conversation

defuncart
Copy link

@defuncart defuncart commented Nov 18, 2022

Currently opening this PR as a draft in order to receive feedback on the api

Can be used as follows:

_controller = VideoPlayerController.asset(
  'assets/Butterfly-209.mp4',
  videoPlayerOptions: VideoPlayerOptions(
    webOptions: const VideoPlayerWebOptions(
      controlsEnabled: true,
      allowDownload: false,
      allowContextMenu: false,
      allowFullscreen: false,
      allowPlaybackRate: false,
    ),
  ),
);

Bildschirmfoto 2022-11-18 um 10 44 22

Pre-launch Checklist

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

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

@google-cla
Copy link

google-cla bot commented Nov 18, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@stuartmorgan-g
Copy link
Contributor

@ditman Can you take a high-level look at the API changes here?

I think the main question is whether we want to expose things through an options mechanism like this, or do something more direct like exposing the element for direct manipulation. How many more things like this might we want to add later? If we end up duplicating all of a large API surface that would be problematic, but if it's a fairly small set of options this (or the alternate pattern we have in some other plugins of using platform-specific option subclasses) could be reasonable.

@defuncart
Copy link
Author

@ditman Any feedback on the proposed api? I do not see the web options growing substantially over time, maybe options to enable/control video speed and PiP (Picture in Picture).

@defuncart defuncart force-pushed the feature/web-add-options branch from 864f876 to d41b654 Compare December 1, 2022 15:45
@defuncart defuncart force-pushed the feature/web-add-options branch 2 times, most recently from 32ec707 to 78dc4f8 Compare January 14, 2023 12:16
@defuncart
Copy link
Author

@stuartmorgan @ditman Any feedback on the api? I've added options to also disable full screen and not display playback rate.

@defuncart defuncart force-pushed the feature/web-add-options branch from 78dc4f8 to 69b555a Compare January 14, 2023 12:22
@defuncart defuncart marked this pull request as ready for review January 26, 2023 17:46
@defuncart defuncart force-pushed the feature/web-add-options branch from 69b555a to b687716 Compare January 28, 2023 11:20
@camsim99 camsim99 removed their request for review February 6, 2023 17:36
@stuartmorgan-g
Copy link
Contributor

We've just completed the migration of the plugin code to the flutter/packages repository, as described in https://flutter.dev/go/flutter-plugins-repo-migration, and this repository is now being archived. Unfortunately that means that all in-progress PRs here must be moved to flutter/packages.

Please see our instructions for an explanation of how to move your PR, and if you have any issues moving your PR please don't hesitate to reach out in the #hackers-ecosystem channel in Discord.

Our apologies that your PR was caught in this one-time transition. We're aware that it's disruptive in the short term, and appreciate your help in getting us to a better long-term state!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

video_player protect video from download
2 participants