Skip to content

Conversation

GaecKo
Copy link

@GaecKo GaecKo commented Oct 13, 2025

Proposed Changes

Added a button to end workout in the "jump to" section during gym mode.

This button is not accessible from start_page.dart nor session_page.dart in 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 _totalPages attribute in gym_mode.dart, that is calculated in already-existing function _calculatePages().

Screen Recording 2025-10-13 at 23 57 17

Related Issue(s)

Closes #920 : Option to end workout in the "jump to" section

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features) -> doesn't seem to have any existing test files for widgets
  • Set a 100 character limit in your editor/IDE to avoid white space diffs in the PR
    (run dart format .)
  • Updated/added relevant documentation (doc comments with ///). -> doesn't seem to have an existing doc for that part
  • Added relevant reviewers.

Waiting for your feedback to rework on necessary parts :).

this._controller,
this._totalPages, {
required this.exercisePages,
this.hideEndWorkoutButton = false,
Copy link
Member

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?

Copy link
Author

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 :)

Copy link
Member

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

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.

Option to end workout in the "jump to" section

2 participants