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

Feature: Add ability to mark Monthly Funding Goal as a "Max Spending Goal" #1609

Merged
merged 6 commits into from Aug 8, 2019

Conversation

@ajgisler
Copy link
Contributor

commented Jul 21, 2019

This changes the goal to be positive when spending is less then goal amount.

Explanation of Feature:

Gives the user the ability to mark a Monthly Funding Goal category as "Max Spending". This will change categories where the monthly funding goal has not been met to green. Likewise when a category is over the monthly funding goal this will be yellow to reflect overspending of category.

The goal chart color is also updated to reflect spending. Also the text "xx% Complete" is changed to "xx% spent" when category is under spending of the goal.

@joshmadewell joshmadewell changed the title Checkbox was added to Monthly Funding Goal. Feature: Add ability to mark Monthly Funding Goal as a "Max Spending Goal" Jul 22, 2019

@joshmadewell
Copy link
Member

left a comment

@ajgisler This is looking pretty good! I think it may still be a WIP since you have a TODO and a debug statement but an early review never hurts!

If you could also update the PR description to briefly describe your feature, that'd be awesome! Should it highlight a categories' available column as red if the user overspent? Make sure you also test this feature with other features running just to make sure they all work properly. The ones that come to mind are:

Goal Indicator Warning Color
Highlight all Negative Category Balances Red
Unhighlight all Positive Category Balances
Warn When Target Balance is Not Reached
src/extension/utils/spendinggoals.js Outdated Show resolved Hide resolved
}
} else if ($(goalstate).hasClass('cautious')) {
// TODO: update goal chart to reflect overspending.
this.udpateGoalChart('cautious');

This comment has been minimized.

Copy link
@joshmadewell

joshmadewell Jul 22, 2019

Member

should we be reflecting overspending in the available UI as well ?

This comment has been minimized.

Copy link
@ajgisler

ajgisler Jul 24, 2019

Author Contributor

The available column gets updated previously to show overspending or positive. This section is a WIP. Here is where I would like to change the goal chart to show a percent of over spent rather than the current check mark. I have started working on this but dealing with all the variations of currencies is more challenging than I had thought. I think this would be a nice feature but I am not sure it is absolutely necessary. What you think?

ajgisler added 2 commits Jul 24, 2019
@joshmadewell
Copy link
Member

left a comment

This is super close! I think I'm a bit confused on the feature a bit still:

Screen Shot 2019-07-26 at 5 06 08 PM

In this image, there are two categories which have both been assigned as "max spending" goals. The first column has spent more than the goal (150) but the available column doesn't receive any alert (should it be yellow?)

In the second column, I have a goal of $100, I've only spent 78.75 but the highlight is yellow. Shouldn't we make it green since I've not hit my max spending?

The case where I overspend appears to be working properly in the cases I've tested.

spendingGoals.push(subcategoryID);
setSpendingGoals(spendingGoals);
} else if (spendingGoals.contains(subcategoryID)) {
setSpendingGoals(

This comment has been minimized.

Copy link
@joshmadewell

joshmadewell Jul 26, 2019

Member

you can just do the set regardless in the else the filter will remove it if it's in the array.

we also probably wanna call updateAvailableUI because it doesn't look like the highlight goes away if I turn it off.

This comment has been minimized.

Copy link
@ajgisler

ajgisler Jul 26, 2019

Author Contributor

Sorry, I meant to do a push last night but git and I don't seem to be getting along. I believe this issue should be fixed now in the commit I just did. I fixed some issues between some of the other features as well.

This comment has been minimized.

Copy link
@ajgisler

ajgisler Jul 27, 2019

Author Contributor

Your example spending pointed out something really interesting. For the "Splurge Money" this is definitely a bug. I think it has something to do with the available balance being 0.

The problem with the second category is the available column changes based on what is in the budget category vs whats in the activity column.. I think I need to create a new budget that properly tests all these edge cases :)

ajgisler added 3 commits Jul 26, 2019
Fixed several issues with the Available column showing wrong coloring…
… including with other toolkit features.
@joshmadewell

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

@ajgisler From what I can tell this appears to be working as intended now! I just want to double check with you that you're all done and we can get this guy in for a release tomorrow!

@ajgisler

This comment has been minimized.

Copy link
Contributor Author

commented Aug 7, 2019

@joshmadewell For now I am done and if you are happy with it then please add it to the release! Thanks!

@joshmadewell joshmadewell merged commit 909d16b into toolkit-for-ynab:master Aug 8, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Mansarde

This comment has been minimized.

Copy link

commented Aug 11, 2019

Thanks for this nice feature!
Would it be possible to make the "pacing" column change its color as well when this option is enabled?

@ajgisler

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2019

@Mansarde Thanks. I have found a couple other bugs already and will make sure to include a fix for the pacing column.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.