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 "just below" display #315

Merged

Conversation

patrickhlauke
Copy link
Collaborator

@patrickhlauke patrickhlauke commented Oct 11, 2023

Currently, the "just below" text is displayed at inappropriate points when "decimals" is set to 2 (both in unnecessary cases, and - as seen in the screenshot below - just completely wrong cases, as 2.952 is not below 2.95)

screenshot of CCAe showing the ratio 'just below 2.95:1 (2.952:1)'

It should only show if there's a rounding happening one decimal point below the currently set decimal display, and only if it's just below 3, 4.5, or 7.

This PR adds more nuance to the big if used to decide when to show the "just below" bit.

CCAe with the same colours, but now showing just '2.96:1'

(not sure why it decided that 2.952 actually rounds to 2.96 in this case...but nothing that I touched should have influenced that. EDIT: ah, it's likely this which accounts for the difference in rounding between the last release - first screenshot here - and the latest main branch - second screenshot, with my PR applied - 0cee4cc)

Make it dependent on decimals/precision
@patrickhlauke
Copy link
Collaborator Author

unrelated, but when running this locally, it kept showing me an error when trying to open the preferences

ReferenceError: x is not defined

To get it to run locally, I had to uncomment lines 26 to 29 https://github.com/ThePacielloGroup/CCAe/blob/main/src/menu.js#L26 ... is this intentional somehow?

@ferllings
Copy link
Member

unrelated, but when running this locally, it kept showing me an error when trying to open the preferences

ReferenceError: x is not defined

To get it to run locally, I had to uncomment lines 26 to 29 https://github.com/ThePacielloGroup/CCAe/blob/main/src/menu.js#L26 ... is this intentional somehow?

The commented code is intentional, but the error is not :)

@ferllings
Copy link
Member

(not sure why it decided that 2.952 actually rounds to 2.96 in this case...but nothing that I touched should have influenced that. EDIT: ah, it's likely this which accounts for the difference in rounding between the last release - first screenshot here - and the latest main branch - second screenshot, with my PR applied - 0cee4cc)

Might be related to my recent modification:
0cee4cc

I'll merge and to do some proper testing later, to see what I'm missing.

@ferllings ferllings merged commit 617164f into ThePacielloGroup:main Oct 11, 2023
@patrickhlauke patrickhlauke deleted the patrickhlauke-fix-just-below branch October 11, 2023 07:11
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.

2 participants