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: Progress bar does not follow drag #789 #798

Conversation

koutaro-masaki
Copy link
Contributor

This PR fixes issue #789 and the following bugs that were uncovered as a result of fixing it:

  • Due to using Duration.zero as an invalid value, the pointer's behavior at the left edge of the progress bar was unnatural.
  • The pointer was able to go beyond the confines of the progress bar.

@diegotori
Copy link
Collaborator

There's one lint issue that needs to be addressed.

Otherwise, LGTM.

@koutaro-masaki
Copy link
Contributor Author

That is not related to the changes in this pull request. It appears that there is a lint issue in the code of the master branch.

@diegotori
Copy link
Collaborator

Still though. Since I have release authority on this repo, I need to be able to have everything green prior to delaying it to pub.dev.

Once your changes are merged (along with the lint issue), then every other PR will have to re-sync to that lint fix.

Duration videoDuration,
Offset? globalPosition,
) {
if (globalPosition == null) return Duration.zero;
if (globalPosition == null) return null;
Copy link
Collaborator

@diegotori diegotori Nov 27, 2023

Choose a reason for hiding this comment

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

If you're already using Duration.zero as a default value when calling this method, wouldn't it make sense to keep this line instead of returning null here?

Copy link
Contributor Author

@koutaro-masaki koutaro-masaki Nov 27, 2023

Choose a reason for hiding this comment

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

This function is also used here, and this property must distinguish between Duration.zero (pointer dragged to the left edge) and null (not dragged).

final double playedPartPercent = (draggableValue != null

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you can add comments based on the above in the code, so that a reader of this line would understand why it needs to return null, that would be very great. Thanks.

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 2ac2ff8 into fluttercommunity:master Nov 27, 2023
2 checks passed
@koutaro-masaki koutaro-masaki deleted the fix/progress-bar-does-not-follow-drag branch November 27, 2023 14:00
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.

2 participants