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

[video_player] Added the setCaptionOffset #3275

Merged
fluttergithubbot merged 40 commits into from
Jan 26, 2022
Merged

[video_player] Added the setCaptionOffset #3275

fluttergithubbot merged 40 commits into from
Jan 26, 2022

Conversation

@vanlooverenkoen
Copy link
Contributor

@vanlooverenkoen vanlooverenkoen commented Nov 18, 2020

Description

By adding 1 extra method & 1 extra field to maintain the caption offset so it can be used to offset the caption field that already existed.

This pr only includes changes to the dart code so it will be available on Android/iOS/Web/(Desktop)

Related Issues

Adds this new feature: flutter/flutter#67762

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.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the [Flutter Style Guide].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the [CLA].
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@google-cla google-cla bot added the cla: yes label Nov 18, 2020
@vanlooverenkoen vanlooverenkoen mentioned this pull request Nov 18, 2020
Closed
@vanlooverenkoen
Copy link
Contributor Author

vanlooverenkoen commented Dec 10, 2020

@cyanglaz or @iskakaushik can you help me with the submit-queue build? How do I fix this? I have been using this pr for 22 days, without any problems in production.

@cyanglaz
Copy link
Contributor

cyanglaz commented Dec 10, 2020

@vanlooverenkoen submit queue means the tree is red on master, it's nothing to do with your PR :) We are working on make it green soon.

@vanlooverenkoen
Copy link
Contributor Author

vanlooverenkoen commented Dec 27, 2020

@cyanglaz or @iskakaushik I have fixed the merge conflicts. Is it possible to review now we have a green master branch?

@vanlooverenkoen
Copy link
Contributor Author

vanlooverenkoen commented Jan 8, 2021

@Salakar & @rrousselGit can you maybe review this one as well? I am using my git fork for almost 3 months now.

@rrousselGit
Copy link

rrousselGit commented Jan 8, 2021

Sorry for the delay. At the moment we are focusing on non-nullable types, so new features aren't a priority

@vanlooverenkoen
Copy link
Contributor Author

vanlooverenkoen commented Jan 8, 2021

I already fixed the non-nullable merge conflicts. So this pr is already non-nullable supported.

But I get that non-nullable is a priority. Good to know that it is for this plugin. It should be the case for all packages.

Any idea when this could be reviewed or merged?

@vanlooverenkoen
Merge pull request #5 from flutter/master
246a042 
Master merge
@vanlooverenkoen
Merge branch 'master' into feature/#67762-caption-offset
30264a8 
# Conflicts:
#	packages/video_player/video_player/CHANGELOG.md
#	packages/video_player/video_player/pubspec.yaml
@vanlooverenkoen
Fixed head tag
0000745
@vanlooverenkoen
Copy link
Contributor Author

vanlooverenkoen commented Mar 12, 2021

@cyanglaz @rrousselGit can you review this pr? I have merged everything from master. and now all checks have passed

@vanlooverenkoen
Merge pull request #6 from flutter/master
7570933 
Master
@vanlooverenkoen
Merge branch 'master' into feature/#67762-caption-offset
c1162f5 
# Conflicts:
#	packages/video_player/video_player/CHANGELOG.md
#	packages/video_player/video_player/pubspec.yaml
@vanlooverenkoen
Fixed version number
2c170e8
@vanlooverenkoen
Copy link
Contributor Author

vanlooverenkoen commented Jan 7, 2022

@stuartmorgan updated the last comments & fixed the merge conflicts it seems that the submit-queue is broken? Can I do something about that?

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jan 8, 2022

it seems that the submit-queue is broken?

submit-queue is a (unfortunately named) live status check of whether the tree is currently broken; it's there to prevent PRs from landing while the tree is broken. It'll become green automatically without any action once the tree re-opens.

stuartmorgan-g
stuartmorgan-g suggested changes Jan 8, 2022
View reviewed changes
@vanlooverenkoen
Copy link
Contributor Author

vanlooverenkoen commented Jan 14, 2022

@stuartmorgan fixed the latests merge conflicts & comments. Can you review again?

stuartmorgan-g
stuartmorgan-g approved these changes Jan 14, 2022
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with a few final comment nits.

Adding @bparrishMines for final review. (@cyanglaz you're welcome to do it instead if you're interested.)

@stuartmorgan-g stuartmorgan-g removed the request for review from iskakaushik January 14, 2022 20:21
bparrishMines
bparrishMines approved these changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

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

LGTM

@stuartmorgan-g stuartmorgan-g added waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. last mile labels Jan 26, 2022
@fluttergithubbot fluttergithubbot merged commit eb6e912 into flutter:main Jan 26, 2022
@vanlooverenkoen vanlooverenkoen deleted the feature/#67762-caption-offset branch January 27, 2022 08:45
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 27, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 27, 2022
@engine-flutter-autoroll
eb6e912 [video_player] Added the setCaptionOffset (flutter/plugins#3275)
e458dc5
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 27, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 27, 2022
@engine-flutter-autoroll
eb6e912 [video_player] Added the setCaptionOffset (flutter/plugins#3275)
ed63c6b
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 28, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 28, 2022
@engine-flutter-autoroll
eb6e912 [video_player] Added the setCaptionOffset (flutter/plugins#3275)
ca6da26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cyanglaz cyanglaz Awaiting requested review from cyanglaz

2 more reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

@bparrishMines bparrishMines bparrishMines approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes p: video_player waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@vanlooverenkoen @cyanglaz @rrousselGit @stuartmorgan-g @bparrishMines @fluttergithubbot @godofredoc