-
Notifications
You must be signed in to change notification settings - Fork 90
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(javascript-calculator): removing +0 and -0 from the expression displayed on calculator. #380
base: main
Are you sure you want to change the base?
fix(javascript-calculator): removing +0 and -0 from the expression displayed on calculator. #380
Conversation
Hey maintainers, |
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.
Hi @CallmeHongmaybe, thanks for your patience.
I took a look and it seems like this is off to a good start. However, I did notice a couple of issues.
I just took a look and this looks like a great start. While I prefer seeing the entire equation printed out after the "=" is pressed, I can see how stripping out most zeros can make things look much cleaner for things like 0+0+0+3
, which now shows as 0+3
.
However, when I was doing some testing, I noticed some issues with floating point arithmetic. For example, 0.2 + 0.3
throws Uncaught SyntaxError: Unexpected number
.
When I add in some console logs, it looks like expression
is 0.2.3
right before it gets passed to eval
, which leads to the error.
Also, we could look at adding @spiritanand as a contributor manually.
But if @spiritanand can push to your branch, that would probably be easier. Or @spiritanand could also start a review and leave suggestions for possible fixes. If those suggestions are committed, then they would get credit, too.
expression = expression | ||
.replace(/x/g, '*') | ||
.replace(/‑/g, '-') | ||
|
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.
You could probably remove the spaces for here and above.
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.
Will do.
Also can you help with the tests ? Because I'm currently on a stretch.
@scissorsneedfoodtoo thanks for the edge case. |
Such as floats like 0.x in expressions
Checklist:
Update index.md
)Partially closes #49045
With the 6 trailing zeros gone, I think it's still useful to remove any arithmetic involving zeros. That way the expression looks cleaner and the fix can even handle cases where the user types
--0
.