Skip to content
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

goalvc shows also a goal's rate #611

Closed

Conversation

krugerk
Copy link
Contributor

@krugerk krugerk commented Jan 6, 2025

Summary

In its current form, the app neither shows the deltas/totals nor does it show a goal's rate. Thus it takes guess work from looking at the graph to determine how many units are needed for a number of safe days.

(Deltas in #547)
This is a basic implementation that shows the goal rate, consisting of gunits, rate, and gunits.
The three attributes have been added to the Goal model and to the definition for Core Data. A new label has been introduced on the goalvc, between the graph and the recent datapoints, showing the rate.

For UI changes including screenshots of before and after is great.
Screenshot 2025-01-06 at 18 47 00

Validation

ran app in simulator

Issues

Fixes #557

  • Handle also the case when rate is not set (goal is defined by date and value)

@krugerk krugerk marked this pull request as ready for review January 6, 2025 23:27
@krugerk krugerk requested a review from a team as a code owner January 6, 2025 23:27
@krugerk krugerk requested review from theospears and removed request for a team January 6, 2025 23:27
@krugerk krugerk force-pushed the feature/557-show-the-goal-rate branch from b7d855d to f6f524f Compare January 7, 2025 11:13
/// Unix timestamp (in seconds) of the goal date.
@NSManaged public var goalDateRaw: NSNumber?
/// Goal value — the number the bright red line will eventually reach. E.g., 70 kilograms.
@NSManaged public var goalValueRaw: NSNumber?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's work out how to do this without the Raw values and separate wrappers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we know, the commitment dial allows specifying any two of the three. At the goal level, two of goalDate, goalVal, and rate will be defined and the other one null. It might be handy to have this as an enum or similar:

enum CommitmentType {
    case targetValueAndDate
    case targetValueAndRate
    case targetDateAndRate
}

And we would still need some transformer for storing such a thing in core data. I think though just for displaying the rate, the app does not really need to know which commitment type is in use. At least I checked on the Android app and that is what it seemed to me.
Goal.mathishard contains the three values, regardless of which two had been set via the commitment dial.
Using that I might have just gone with Double instead of NSNumber?. However, the api doc also mentions mathishard itself can be null and to handle this and use the fallbacks, any of which could be null, I went with the NSNumber?.

if let calculatedGoalDateGoalValueAndGoalRate, calculatedGoalDateGoalValueAndGoalRate.count == 3 {
self.goalDate = calculatedGoalDateGoalValueAndGoalRate[0].doubleValue
self.goalValue = calculatedGoalDateGoalValueAndGoalRate[1].doubleValue
self.goalRate = calculatedGoalDateGoalValueAndGoalRate[2].doubleValue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this approach is what this is the rate for. From the docs:

rate (number): The slope of the (final section of the) bright red line.

This means it can easily not be the gradient for the graph section you are currently in. For that reason I think the due by table approach makes more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As is, it would show what the Android app shows.

@theospears
Copy link
Collaborator

I'm going to close this out because I think we should be exploring due by table approaches rather than this one.

@theospears theospears closed this Jan 7, 2025
@krugerk
Copy link
Contributor Author

krugerk commented Jan 7, 2025

I'm going to close this out because I think we should be exploring due by table approaches rather than this one.

The Android App shows both the rates and the due-by table with its deltas.

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.

show a goal's rate
2 participants