-
Notifications
You must be signed in to change notification settings - Fork 942
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
Issue 1316: Skip measurable habit #1319
Issue 1316: Skip measurable habit #1319
Conversation
Updating forked dev
…re/case_1316_skip_measurable_habit
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.
Very nice, thanks!
uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/Entry.kt
Outdated
Show resolved
Hide resolved
...ts-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/HistoryCard.kt
Outdated
Show resolved
Hide resolved
The ktlint stuff can be a bit annoying, you can have a look at https://github.com/iSoron/uhabits/blob/dev/docs/GUIDELINES.md#code-style to make it less annoying. I use the commit pre-hook personally. |
Hi @kalina559, thank you for the PR. Besides the comments above, I have a few other questions:
|
Hi @iSoron,
|
For scores, I suggest copying the behavior from boolean habits: a For the target chart, one thing that may make sense is to reduce the total. For example, if you have a daily numerical habit with target of 10, then the chart by default would show a monthly target of 300 (for a 30-day month). However, if you skip one day, the monthly target for that month could drop to 290. This might be difficult to implement, though. We would need to count the number of skips in a certain interval. Regarding skips in non-daily numerical habits, I suggest disabling them for now until we understand that use case better. They don't seem to make a lot of sense to me at this point, but I'm open to suggestions. |
As you explained, skip should reduce the multi-day targets. so if I have a target "150 per week" and I have a skip day in that week, the target should be 6/7 =(7-skip_days)/7 of that number, so it is really what you already suggested, only the user-supplied value is on a different scale (weekly given directly instead of calculated as 7*daily etc.). It could be argued that the skip supports laziness, because if I intended to achieve the cumulative number not by doing it daily anyway, skip could be used to lower the target. But honesty is always up to the user, and if I mark three days as skip because I was bedridden sick it is correct that my jogging target for that week should only be 4/7 of when I had been healthy. |
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.
Thank you for the PR, @kalina559. I think it looks mostly good.
@hiqua Please free to add any comments.
...ts-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/NumberButtonView.kt
Show resolved
Hide resolved
uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/ScoreList.kt
Show resolved
Hide resolved
...its-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/TargetCard.kt
Outdated
Show resolved
Hide resolved
uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/EntryList.kt
Outdated
Show resolved
Hide resolved
Looks good to me. Thanks, @kalina559! |
Resolves #1316. My solution is to add a 'SKIP' button to the numerical habit dialog. If it's clicked, the entry value is set to Entry.SKIP / 1000 = 0.003 and it's displayed in the same way as a skipped yes/no habit.