Skip to content

Change video_demo slightly so it works on web.#46719

Merged
fluttergithubbot merged 5 commits intoflutter:masterfrom
ditman:web_video_player_example
Jan 8, 2020
Merged

Change video_demo slightly so it works on web.#46719
fluttergithubbot merged 5 commits intoflutter:masterfrom
ditman:web_video_player_example

Conversation

@ditman
Copy link
Copy Markdown
Member

@ditman ditman commented Dec 10, 2019

Description

Modify video_demo slightly so it compiles and runs on web (with flutter run -d chrome, for example).

This changes:

  • Short-circuit connectivity check, until that plugin becomes available for web.
  • Prevent platform checks if kIsWeb is true.

Related Issues

Tests

Existing tests seem to pass.

$ flutter test
Building flutter tool...
Running "flutter pub get" in flutter_gallery...                     0.5s
00:41 +127 ~17: /usr/local/google/home/dit/github/flutter/examples/flutter_gallery/test/smoke_test.dart: Flutter Gallery app smoke test                     
> VideoDemo initController "butterfly" 
> VideoDemo initController "bee" 
> VideoDemo dispose
< VideoDemo dispose
01:09 +128 ~17: /usr/local/google/home/dit/github/flutter/examples/flutter_gallery/test/smoke_test.dart: Flutter Gallery app smoke test with semantics      
> VideoDemo initController "butterfly" 
> VideoDemo initController "bee" 
> VideoDemo dispose
< VideoDemo dispose
01:10 +129 ~17: All tests passed!

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change. If not, delete the remainder of this section.

@fluttergithubbot fluttergithubbot added d: examples Sample code and demos c: contributor-productivity Team-specific productivity, code health, technical debt. customer: gallery Relating to flutter/gallery repository. Please transfer non-framework issues there. labels Dec 10, 2019
@fluttergithubbot
Copy link
Copy Markdown
Contributor

fluttergithubbot commented Dec 10, 2019

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@ditman ditman mentioned this pull request Dec 10, 2019
Merged
harryterkelsen
harryterkelsen reviewed Dec 10, 2019
View reviewed changes
Comment thread examples/flutter_gallery/lib/demo/video_demo.dart Outdated
Comment thread examples/flutter_gallery/lib/demo/video_demo.dart Outdated
@ditman
Copy link
Copy Markdown
Member Author

ditman commented Dec 11, 2019

cc @rami-a (this may help when migrating the video demo to the gallery v2?)

sigurdm
sigurdm approved these changes Dec 12, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@sigurdm sigurdm left a comment

Choose a reason for hiding this comment

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

LGTM

goderbauer
goderbauer reviewed Jan 7, 2020
View reviewed changes
Comment thread examples/flutter_gallery/pubspec.yaml Outdated
ditman added 5 commits January 7, 2020 13:12
@ditman
Change video_demo slightly so it works on web.
af3104c 
This changes:

* Short-circuit connectivity, until that plugin becomes available for web.
* Prevent platform checks if kIsWeb is true.
@ditman
Address first round of comments.
ab55796
@ditman
Add .gitignore to exclude web artifacts.
066cf05
@ditman
flutter update-packages --force-upgrade
1650d9b 
(After bumping video_player to 0.10.5+1 in
examples/flutter_gallery/pubspec.yaml)
@ditman
Fix analyzer error
1e03092
@ditman
Copy link
Copy Markdown
Member Author

ditman commented Jan 8, 2020

Waiting for @goderbauer to take another look, and for the tree to become green again!

@ditman ditman requested a review from goderbauer January 8, 2020 01:27
goderbauer
goderbauer approved these changes Jan 8, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@fluttergithubbot fluttergithubbot merged commit f65f7f0 into flutter:master Jan 8, 2020
@ditman
Copy link
Copy Markdown
Member Author

ditman commented Jan 8, 2020

Yay! Thanks @fluttergithubbot! :D

@ditman ditman deleted the web_video_player_example branch January 8, 2020 21:25
bkonyi pushed a commit that referenced this pull request Jan 9, 2020
@ditman @bkonyi
Change video_demo slightly so it works on web. (#46719)
e1c0fa8
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@harryterkelsen harryterkelsen harryterkelsen left review comments

@goderbauer goderbauer goderbauer approved these changes

@sigurdm sigurdm sigurdm approved these changes

@iskakaushik iskakaushik Awaiting requested review from iskakaushik

@johnpryan johnpryan Awaiting requested review from johnpryan

Assignees

No one assigned

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. customer: gallery Relating to flutter/gallery repository. Please transfer non-framework issues there. d: examples Sample code and demos

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ditman @fluttergithubbot @goderbauer @harryterkelsen @sigurdm @googlebot