Skip to content

WIP: Default weight unit#1181

Open
jmccaull wants to merge 3 commits into
wger-project:masterfrom
jmccaull:default-weight
Open

WIP: Default weight unit#1181
jmccaull wants to merge 3 commits into
wger-project:masterfrom
jmccaull:default-weight

Conversation

@jmccaull
Copy link
Copy Markdown

Initial PR to solicit feedback

Proposed Changes

  • refactor main app to provide PlateCalculatorNotifier as a dependency for RoutineProvider
  • remove default weight unit from set config data
  • adjust RoutineProvider.defaultWeightUnit to use PlateCalculatorNotifier's metric setting
  • adjust RoutineProvider logic to utilize defaultWeightUnit when none provided

Related Issue(s)

Closes #1180

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Set a 100-character limit to avoid white space diffs (run dart format .)

@jmccaull
Copy link
Copy Markdown
Author

Its been a while since I've done any dart and Riverpod is all new to me so I'm not sure if this is the right way to handle the provider setup. I did not think it made sense to refactor all the providers needed for RoutineProvider to be fully Riverpod in a single pull request.

I'm also looking for feedback on the set_config_data changes. I personally don't think DTO like objects should be selecting a default value for a field that is indicated as null-able, but it seems like an established pattern here. If this change goes forward, does it make sense to also refactor the recent log changes here?

@rolandgeider
Copy link
Copy Markdown
Member

Hi! Sorry for the late reply.

There's another PR that I'm working on that is migrating the providers to riverpod, so that problem will disappear in the future.

And as for the PR, I'm not sure if making the plate calculator the source of truth is the correct way. There is already a flag in the profile that handles this (but as you've probably noticed, hasn't the biggest impact in the app) and I would tend to use that (also for the initial value for the plate calculator itself).

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.

Use plate selection to imply default weight unit

2 participants