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

Fix the logic of the Center Play Button icon selection #828

Merged
merged 8 commits into from
Aug 5, 2024

Conversation

EmreDET
Copy link
Contributor

@EmreDET EmreDET commented Apr 12, 2024

Hello, this is a small fix. We saw that the plugin has a logical error whenever we did not initialize the video, the icon for the play button would show the "reload"-icon. This happens because: _latestValue.position >= _latestValue.duration indicates, that if duration was 0, the initial position would also be 0, therefore the video player icon logic falsely assumed that the video needs to be "reloaded" from the beginning. With this simple fix && _latestValue.duration.inSeconds > 0 we ensure, that it should only be treated as "isFinished" if the duration is non-zero, therefore was initialized. If needed, we can even do && _latestValue.duration.inMilliseconds > 0 or _latestValue.duration.inMicroseconds > 0, for further precision.
Please correct me if I have made a mistake.

Thank you for your time and efforts.
Emre Y.

@MrJohnDev
Copy link

v

Good!

@EmreDET
Copy link
Contributor Author

EmreDET commented Aug 1, 2024

Will this be merged soon? :)

In the meantime for others, here is my fork: https://github.com/EmreDET/chewie

chewie:
    git:
      url: https://github.com/EmreDET/chewie.git
      ref: master

@diegotori
Copy link
Collaborator

@EmreDET Thank you for your contribution.

Please confirm that this issue also applies to Cupertino as well. If it does, then apply the same fix there as well.

The earliest I'll be able to deploy this will be on Monday.

@EmreDET
Copy link
Contributor Author

EmreDET commented Aug 1, 2024

Hey @diegotori, thank you for your quick response.
Indeed, the same behavior was falsely in CupertinoControls & MaterialDesktopControls, I fixed them as well.
I also recreated the iOS example app, so it runs on new devices (iPhone 15 Pro, iOS 17.4).
Lastly, I added a test for that exact behavior.

Please let me know if I did something wrong. Thank you.
Emre Y.

Copy link
Collaborator

@diegotori diegotori left a comment

Choose a reason for hiding this comment

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

LGTM.

@diegotori diegotori merged commit 039c793 into fluttercommunity:master Aug 5, 2024
3 checks passed
diegotori added a commit that referenced this pull request Aug 5, 2024
@diegotori
Copy link
Collaborator

@EmreDET published in version 1.8.3.

ganeshh123 pushed a commit to greg-mat/chewie that referenced this pull request Aug 24, 2024
lg8294 added a commit to lg8294/chewie that referenced this pull request Aug 27, 2024
* commit 'f58a65003cb3650e7c85e7bc4907454d03bf68a3':
  chewie, version 1.8.3. Adds PR fluttercommunity#828.
  Updated workflows so that they now run tests as a result of this PR.
  format
  add small test for verification
  add fix for cupertino & desktop
  recreate ios project to make it runnable again
  Update material_controls.dart
  fix: isFinished is true only if initialized else false

# Conflicts:
#	example/ios/Runner.xcodeproj/project.pbxproj
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants