-
-
Notifications
You must be signed in to change notification settings - Fork 336
Feature/end_workout_button in "jump to" section #955
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
Feature/end_workout_button in "jump to" section #955
Conversation
| this._controller, | ||
| this._totalPages, { | ||
| required this.exercisePages, | ||
| this.hideEndWorkoutButton = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when we hide the jump button we don't care about the total pages. Perhaps we could make it nullable or so and hide it in that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I see!
If we were to do that, I would need to remove the this.hideEndWorkoutButton attribute, in order to avoid inconsistencies between this.hideEndWorkoutButtonand _totalPages. Such inconsistency would happen if this.hideEndWorkoutButton is false (default value) but _totalPages is also null. In such case an error would be raised as the button would be shown but missing the require non-null value of _totalPages to work.
If we only keep the _totalPages attribute, its value being null or not would define if the button is shown or not. It works and is consistent, but NavigationHeader behaviors may not be as clear as currently with this.hideEndWorkoutButton in the constructor.
No solution is ideal. Current one gives extra "useless" information, but that information is anyway computed beforehand, so including it doesn't create extra calculation.
@rolandgeider waiting for your opinion :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, both are slightly unoptimal, but I personally would go with only having the totalPages and just adding a comment
|
Hey, what about now @rolandgeider ? I have adapted the constructor so |
|
yeah I think so too |
|
🚀 |
Proposed Changes
Added a button to end workout in the "jump to" section during gym mode.
This button is not accessible from
start_page.dartnorsession_page.dartin the jump_to section. This is an arbitrary choice, as ending a workout without having started it, or ending an ended workout doesn't seem logical.Other than that, the button is accessible from other pages of the gym mode.
In order for the end workout button to work, using the
_controller.animateToPage(...), I needed to know how many pages there were. I therefore added a_totalPagesattribute ingym_mode.dart, that is calculated in already-existing function_calculatePages().Related Issue(s)
Closes #920 : Option to end workout in the "jump to" section
Please check that the PR fulfills these requirements
(run
dart format .)///). -> doesn't seem to have an existing doc for that partWaiting for your feedback to rework on necessary parts :).