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

Fix Issues #102 and #106 #108

Closed
wants to merge 2 commits into from
Closed

Fix Issues #102 and #106 #108

wants to merge 2 commits into from

Conversation

DJTai
Copy link

@DJTai DJTai commented May 19, 2018

No description provided.

David T added 2 commits May 19, 2018 11:44
Add Three "Servings" For Exercise Instead Of One

- Also adjusted serving sizes accordingly (90 and 40 minutes to 30 and 14)
- Increased max servings from 24 to 26
Inertial Scrolling

- added "nestedScrollingEnabled" and set to false for affected views.
@slavick
Copy link
Member

slavick commented Jul 28, 2018

@DJTai,

Thank you for your fix to issue #106, but I had to add it manually as the nestedScrollingEnabled xml property is only available on SDK level 21 and higher. This required the creation of a layout-v21 directory and duplicating the activity_food_info.xml layout file inside it (with your updates). If the property were to have been included in the base layout, it would have caused crashes for all users whose SDK level was less than 21.

As for your fix to issue #102, it's not simply a matter of increasing the number of checkboxes. As the app currently functions, all of the checkboxes are worth 1 point towards the daily total. This results in 24 total possible points for the day. Your change would have changed this to 26 and affected the calculations for all users. All users would have lost their gold stars for days in which they completed all of their servings. This change would have also made the app's checklist differ from that in How Not to Die. I think it's important that the checklist in the book matches the checklist in the app.

Thank you for raising this PR, I'm going to mark issue #106 fixed.

@slavick slavick closed this Jul 28, 2018
@miyl
Copy link

miyl commented Aug 17, 2018

As far as I can see the book has one box for exercise? If you want the app to mirror the book 1:1 why is the issue open at all?

Maybe the issue could be made more explicit so it's clearer what you want. Regarding gold stars I guess the update must ensure that everyone with 1 checkmark (full) currently in exercise anywhere should be updated to 3 (full) in the new version?

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.

None yet

3 participants