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

Handle fractional ingredients (Fixes #2004) #2421

Merged
merged 4 commits into from
Sep 12, 2024

Conversation

Weissnix4711
Copy link
Contributor

@Weissnix4711 Weissnix4711 commented Jul 13, 2024

Topic and Scope

My solution to #2004. This handles 16ths, else falls back to decimal. I think this is a nice balance between using some fractions, without ending up with stupid denominators.

Concerns/issues

Will only work if the initial ingredient is given as a fraction. If it's a decimal, or just a whole number, will still remain decimal. Maybe it would make sense to convert everything to fractions where possible, but then maybe not. Perhaps being able to change the behavior in settings would be useful, idk. Also, by design does not do 1/3, 1/5, etc. I don't mind this, in fact I would prefer this way, but I guess in the end it's not up to me.

Formal requirements

There are some formal requirements that should be satisfied. Please mark those by checking the corresponding box.

  • I did check that the app can still be opened and does not throw any browser logs
  • I created tests for newly added PHP code (check this if no PHP changes were made)
  • I updated the OpenAPI specs and added an entry to the API changelog (check if API was not modified)
  • I notified the matrix channel if I introduced an API change

@seyfeb
Copy link
Collaborator

seyfeb commented Jul 15, 2024

Thank you for your contribution! Smaller than 1/16 probably does not make any sense indeed. 1/3 and 1/5 are debatable, though. However, as a starting point, having any conversion is better than none. In future iterations there could even be a toggle in the recipe view or some options in the configuration but that is not required immediately.

I did not check the details yet, but one thing that comes to my mind is, there are ASCII/Unicode symbols for fractions like ¾. I think they should be handled, too. Maybe not all of them, but the most common ones?

A quick Google search turned up this reference:

https://www.compart.com/de/unicode/decomposition/%3Cfraction%3E

@Weissnix4711
Copy link
Contributor Author

Done. Should now parse unicode fractions, but will always display the output as 'normal' text (ie. using digits and a slash). I guess it would be possible to display the output with unicode fractions too, but at least for me they don't always display too nice in the browser.

Handle 16ths during recalculation of ingredients, else use decimal

Signed-off-by: Christian Wolf <[email protected]>
Copy link

github-actions bot commented Sep 12, 2024

Test Results

   12 files    584 suites   1m 37s ⏱️
  575 tests   575 ✅ 0 💤 0 ❌
2 300 runs  2 299 ✅ 1 💤 0 ❌

Results for commit 8a39540.

♻️ This comment has been updated with latest results.

Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Copy link
Collaborator

@christianlupus christianlupus left a comment

Choose a reason for hiding this comment

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

Although there is a small bug there, this is better than before.

@christianlupus christianlupus linked an issue Sep 12, 2024 that may be closed by this pull request
@christianlupus christianlupus merged commit 78c523a into nextcloud:master Sep 12, 2024
19 checks passed
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.

Handle fractions nicely during recalculations
3 participants