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

Weight entry QoL improvements #632

Merged
2 commits merged into from
Sep 7, 2024

Conversation

sizzlesloth
Copy link
Contributor

@sizzlesloth sizzlesloth commented Sep 5, 2024

Description (Proposed Changes)

  • Use current DateTime when copying WeightEntry instead of using previous weight.
  • Increment buttons increment weight by 0.1 and round to one decimal place to prevent floating point accuracy (rounding) issues.

Link to the issue :

Checklist

  • Set a 100 character limit in your editor/IDE to avoid white space diffs in the PR
  • Tests for the changes have been added (for bug fixes / features)
  • Added yourself to AUTHORS.md
  • Updated/added relevant documentation (doc comments with ///).
  • Added relevant reviewers.

Increment weight by 0.1 and round to one decimal place to prevent floating point accuracy (rounding) issues.
Removed whitespace.
@Dieterbe Dieterbe self-requested a review September 7, 2024 16:22
@@ -43,7 +43,7 @@ class WeightEntry {
WeightEntry copyWith({int? id, int? weight, DateTime? date}) => WeightEntry(
id: id,
weight: weight ?? this.weight,
date: date ?? this.date,
date: date ?? DateTime.now(),
Copy link
Contributor

Choose a reason for hiding this comment

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

when you call copyWith(), you expect that it copies the model, and overrides those fields that are given.
therefore this change doesn't make sense. Instead, the callers should just provide whichever date they want to use (DateTime.now())

@Dieterbe
Copy link
Contributor

Dieterbe commented Sep 7, 2024

also flutter test is broken.

your PR is a good start, it just needs some tweaks. i will finish it.

@Dieterbe Dieterbe mentioned this pull request Sep 7, 2024
@Dieterbe Dieterbe closed this pull request by merging all changes into wger-project:master in 96faf89 Sep 7, 2024
@Dieterbe
Copy link
Contributor

Dieterbe commented Sep 7, 2024

@sizzlesloth thanks! i merged your changes via the PR above.

@sizzlesloth
Copy link
Contributor Author

Thank you very much! :)

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