open-trackers / Gym-Routine-Tracker-Watch-App

Minimalist gym workout tracker, as independent watchOS app
https://open-trackers.github.io/grt
Mozilla Public License 2.0
36 stars 8 forks source link

Support for user-configurable defaults for sets/reps/intensity/units #1

Closed reedes closed 1 year ago

reedes commented 1 year ago

Presently they're hard-coded at 3/12/30/none.

Could be based on App Storage or in database.

TahhanCoding commented 1 year ago

I will be working on this next week isa.

reedes commented 1 year ago

Thanks! Watch for a big merge coming within a few days. (The mountain branch with history logging.)

reedes commented 1 year ago

Thinking about this...

reedes commented 1 year ago

Note that I reworked settings form view to support export on iOS.

TahhanCoding commented 1 year ago

I am going along with AppStorage approach, then defaults is loaded every time a new exercise is to be created. I haven't make the connect yet, I am making the view of default settings, but, there is a problem with Picker. defaultUnit as I named it, doesn't get value from the picker. And the picker wheel every time returns to a "none" state. I am working on it. Just to keep things updated.

Simulator Screen Recording - Apple Watch Series 8 (41mm) - 2023-01-18 at 11 59 13

reedes commented 1 year ago

Great to hear!

I'll be traveling through the 28th and will be able to take a close look then.

TahhanCoding commented 1 year ago

Hope things are going will, I managed to solve the problem alhamdulillah, AppStorage can't save a custom type so I made a turn-around and picking did work:

   @AppStorage("defaultUnitStored") public var defaultUnitStored = "Kg"
   @State var defaultUnit = Units.kilograms

Now I should work on making the creating of new exercise loads default settings. Note that there is a problem regarding the WatchOS ScrollView itself that causes this message to appear when loading SettingsForm

Screenshot 2023-01-19 at 2 13 34 PM

check: https://stackoverflow.com/questions/69873690/why-watchos-picker-always-shows-scrollview-contentoffset-binding-has-been-read

TahhanCoding commented 1 year ago

https://github.com/gym-routine-tracker/GroutUI/pull/9#issue-1554411186

Have a nice day.

TahhanCoding commented 1 year ago

https://user-images.githubusercontent.com/97001250/214384021-61a01418-9e0b-45b2-b0cd-f97e4a8dd6de.mp4

reedes commented 1 year ago

I'm back now and will take a look at your pull request on Monday.

reedes commented 1 year ago

Hi Ahmed

Thanks again for the help in moving this project forward. Some suggestions from reviewing your code:

Regarding storing default units in AppStorage: rather than store as a String inside DefaultUnitStored, try extending Units with RawRepresentable as I did with Date. That can avoid a separate @State storage and avoid the onChange complexities. StackOverflow and other sites should have other examples.

Regarding the view containing the stepper controls, avoid wrapping in a VStack, as it'll interfere with rendering the controls in a Form. If necessary (due to SwiftUI limitations), break out into separate views as I did with ExerciseDetail and elsewhere.

In the View body generally avoid:

This is more advanced refactoring and can be dealt with later, but ideally the same views used to set the defaults can be used to set the values in ExerciseDetail. (I'm big on eliminating duplicate views/code where feasible.)

Also, I run swiftformat (https://github.com/nicklockwood/SwiftFormat or homebrew) on the whole workspace before committing, to reduce diff noise, as seen in SettingsForm in the PR.

Reed

TahhanCoding commented 1 year ago

I'd like to mention that I am very happy for your last comment as this is Why I decided to contribute to opensource. To have someone help me improve my code while adding value. So thanks for your patience and I will take these points into account. I will work on this asap isa.

reedes commented 1 year ago

Glad to help. Though I wouldn't say I've mastered SwiftUI, I've learned quite a bit over several projects.

Take your time. I've pushed out 1.6 today and will be troubleshooting what may be a CloudKit syncing problem.

reedes commented 1 year ago

Fixed the CloudKit syncing problem! (Fix in GroutLib)

reedes commented 1 year ago

Example (on a temporary branch) of modifying an existing view so that it can be used with both ExerciseDetail and ExerciseDefaults...

https://github.com/gym-routine-tracker/GroutUI/commit/835ffcdd42a36011bcea13d582d93a7a0a817407

This is an example of avoiding duplicate code and providing the user a consistent interface.

TahhanCoding commented 1 year ago
TahhanCoding commented 1 year ago

I installed and ran SwiftFormat as well as adding foregroundStyle(.tint). I checked the link you provided for avoiding the duplicate code but I couldn't understand it so if you could write how to do it in plain text it will be better for me. I think opening a new issue for refactoring the views would be good.

reedes commented 1 year ago

Ahmed, thanks for the update!

Big change of plans which will unfortunately delay this feature: I started working on a daily calorie counter app.

As the new app will share at least 25% of the code with GRT, I'll need to refactor GRT to use the new packages.

I expect to have the new app out and GRT refactoring done by end of the month.

(Though I'll push out a bug fix release of GRT next week.)

TahhanCoding commented 1 year ago

An idea came to me, I think removing settings view entirely would be more user friendly. And when user create new exercise, attributes he selects are stored as default attributes and loaded when creates another one.

reedes commented 1 year ago

That might work. Good idea!

I'm all for simplifying the UI where feasible.

Let's revisit once I get the new app out and the refactoring completed, in a few weeks.

reedes commented 1 year ago

I've added this feature as part of a huge rework of code. Defaults stored in new AppSetting entity. On the 'mega' branch.

https://github.com/gym-routine-tracker/GroutUI/commit/c2a7f71c77ce048fd21aaf9d5ab5d03d80194396#diff-0dc777e2683475f44459d4f60549795f74480b6501d6453fdbf52c3b904cd6e0

reedes commented 1 year ago

Re your proposal of using the last config for the new exercise, I worried that it might surprise a user, as in not being expected behavior.

I added a route to a dedicated view for modifying the Exercise defaults, so as to not clutter up the existing settings view. The default values are stored in new AppSetting entity that I mirrored from new DCT app, so should share across devices.

See what you think and thanks again for the input and moving the enhancement forward!

TahhanCoding commented 1 year ago

Having a view for modifying default settings is more common. So for consistency and UX I agree with your opinion. I will check issues soon isa to start working on one of them. Have a nice day.

reedes commented 1 year ago

Everything should be updated/closed for the x.y.1 releases I submitted for review tonight.

There's a bunch of enhancements that look fun, such as watch complications. See what interests you!