-
Notifications
You must be signed in to change notification settings - Fork 649
Address feedback from initial review of application flow #1678
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
Conversation
- bullets for selected benefits - format as $ USD with commas for thousands separators:w
capacities_met = any( | ||
[ | ||
any([not b.has_capacity for b in benefits_qs.filter(program=p)]) | ||
for p in programs | ||
] | ||
) |
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.
Hm, I didn't get why we have to filter the benefits_qs
by program
to check if there's one of them without available capacity. Can't we refactor the code to:
capacities_met = any( | |
[ | |
any([not b.has_capacity for b in benefits_qs.filter(program=p)]) | |
for p in programs | |
] | |
) | |
capacities_met = all([b.has_capacity for b in benefits_qs]) |
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.
I did this primarily since I anticipate that at some point we will have programs that are no longer active. a bit premature.
@ewdurbin I updated this PR fixing the remaining 2 items (benefit description + selection highlight bug). Feel free to review, test and merge this PR if nothing else pops up. I'm attaching 2 screen shots as quick demos: |
dd7bc62
to
89cc56e
Compare
2fbd486
to
b743a59
Compare
benefit description + selection highlight bug has reappeared. Explanation: when selecting a package some 'benefits' do not highlight in blue' |
@JackieAugustine thanks, but this is probably related to #1687, not this Pull Request. |
@JackieAugustine fix for that is introduced in #1688 |
Uh oh!
There was an error while loading. Please reload this page.