-
Notifications
You must be signed in to change notification settings - Fork 945
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
Implement numerical habits with AT_MOST target type #1101
Conversation
82aa05a
to
cbf6fad
Compare
cbf6fad
to
804edfa
Compare
b9881b4
to
1a56260
Compare
Thanks for working on this, @KristianTashkov. A few thoughts:
|
Yeah I can update this. In general the use of color is very inconsistent in the app, especially between numerical history chart and the main screen. If the default value is not zero, then what should the color/text be on no data entered days if you don't have question marks enabled?
In the initial version I started off the habit with a default value of
WDYT? |
As I mentioned on your other PR, I think that ideally the UI should have some kind of toggle button instead of a selection with only two options |
@KristianTashkov Sorry for the delay. I agree with you that the default value should be zero. I also agree that making the default configurable would be a different feature and a different PR. I just think that, if we pick zero as the default value, then a natural consequence is that a completely new AT_MOST habit should have a perfect 100% score. My concrete suggestion is to:
|
@iSoron I picked some of my changes from the other PR (#1106) to handle having a different history chart square. |
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 changes, @KristianTashkov. See some comments below.
uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/Habit.kt
Outdated
Show resolved
Hide resolved
uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/ScoreList.kt
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
Could you please elaborate on this? What do you find inconsistent, and do you have any suggestions to improve it? |
We use color to denote different states of habits in two places: main screen and history screen. YES/NO:Most of it is okay, color means completed, half-color (either not full check or dimmed square in history) means auto success, no color means fail. If you have question marks enabled, there isn't a colorful question mark though, it just gets overridden to an auto check and there is no notion of unknown. If you try to "fail" it just reverts to a success. If color means "success" of some kind, shouldn't we have a colorful question mark to indicate "nothing is entered, but you didn't need to today anyway". Then "failing" it just makes it "auto success" vs "manual success". Numerical
In general it seems yes/no uses color to indicate "success", while numerical uses color to indicate "presence of data". I think color should always indicate success for both, and we can use dimness to indicate whether you have fully reached the target, or only made progress towards it. Doing nothing on that day, or going above your 2 * target should then mean failure, similar to |
9d8de8a
to
d6a7fa3
Compare
@iSoron made the changes you requested and added tests for both |
Thank you for the implementation, @KristianTashkov. I have merged it with some minor UI changes (see the last two commits). I'll comment on the color inconsistencies soon. |
A question mark indicates that the app does not have sufficient data to determine whether you have successfully completed a habit on a given day. Color, on the other hand, indicates that you have successfully completed the habit. If we stick to these definitions, then the concept of a colorful question mark is unclear to me, since the definitions of "color" and "question mark" seem to contradict each other. Regarding auto success, if you have a weekly habit, and you completed it yesterday, then today is unequivocally a success, regardless of today's value; so, even if we don't have data about today's value, the app can still confidently say that you have completed the habit; that's the reason why we don't show a question mark.
The idea here is that light grey means missing data, while dark grey means failure. Note that, if you have question marks enabled, then any zero values are interpreted as missing data. I agree that the current implementation does not follow these conventions. For example, before this PR, we always showed zero values as light grey for numerical habits in the main screen. After this PR, we are always showing zero values as dark grey, regardless of question mark preference. The calendar and history charts are also not following them. PRs to make this more consistent are very welcome.
That's not accurate. Color means success, period. For YES/NO habits, in the calendar, we are using different shades and patterns to indicate different types of success (automatic, manual, skip). |
I think this is a very weird definition of the feature and the option in preferences doesn't say that:
Regardless of whether the day is a success, there is missing data here. We allow a user to add a manual checkmark on a day they weren't expected to, so until they provide us with a checkmark, or confirmation that they didn't do the habit this day, this is missing data to me.
Then why is 5 out of 10 in a numerical |
Perhaps we could clarify the option title, but the expanded description already clarifies that question marks are supposed to differentiate between actual lapses (
Please feel free to open a feature request for that. (I think we might already have one).
That's a bug.
The initial implementation used a color gradient to indicate level of completeness. The main issue is that light colors look too similar to grey, so it was hard to distinguish whether data was missing, or if the data was there, but had value close to zero. This was specially true for some palette colors, such as yellow. If we are using a single dimmed color, it's unclear to me the advantages when compared to just using grey. |
This implements the option to select target type in Habit creation and editing. Screen looks like this:
There are multiple changes to make this work throughout the app. We can pick and choose which to merge or which to change. A list of the changes I'm introducing:
AT_MOST
case. It basically has two thresholds. Below the target you are considered 100% completed. Between the target and twice the target, you are getting linearly less progress. After twice the target you have 0% completenessAT_MOST
.AT_LEAST
implementation is weird there, but tried to capture it's meaning respectively forAT_MOST
In general a good question for us to answer is how do we want to treat not entered values for
AT_MOST
. If we take the0
we have today for numerical habits, it means that anAT_MOST
habit is automatically completed on a not entered day, instead of the usual "auto fail".