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(javascript-calculator): adjust minus unicode, add noreferrer #376

Merged
merged 2 commits into from
Feb 18, 2023

Conversation

Dave2188
Copy link
Contributor

@Dave2188 Dave2188 commented Feb 15, 2023

Checklist:

Partially addresses #49045

I included the closing number for issue 49045 because while working on it, I could not replicate that specific issue. In the process, I discovered that the minus operator was showing as a zero. I changed the character based on the warning and has seemed to fix the issue.

Also was receiving a warning due to missing rel-tag, so I included that as well.

Screenshot 2023-02-14 at 12 00 08 AM

Screenshot 2023-02-14 at 7 35 20 PM

Screenshot 2023-02-14 at 7 31 32 PM

@ShaunSHamilton ShaunSHamilton changed the title fix(Javascript-calculator) Minus operator showing as zero, and rel tag warning on anchor fix(javascript-calculator): minus operator showing as zero, and rel tag warning on anchor Feb 15, 2023
@ShaunSHamilton ShaunSHamilton changed the title fix(javascript-calculator): minus operator showing as zero, and rel tag warning on anchor fix(javascript-calculator): adjust minus unicode, add noreferrer Feb 15, 2023
Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time to contribute.

I am happy with this change. I also see the warning about the uncommon unicode character. However, I do not see a 0 in place of it on the production app.

How/where did you get the 301 = 2?

@Dave2188
Copy link
Contributor Author

Every time I would hit the minus key a zero would appear on the calculator. The functionality was working correctly. The only thing I could think of was that it was having trouble with the character. I'm on a mac with chrome if that could be playing a part.

Copy link
Contributor

@scissorsneedfoodtoo scissorsneedfoodtoo left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time to investigate this issue and for these fixes, @Dave2188.

I can replicate this on my Mac running Chrome, where 9-3=6 appears as 903=6:

image

Firefox doesn't have the same issue, and the - operator is displayed correctly.

The fixes here seem to do the trick. Here are some screenshots from Chrome on my Mac after the fixes:

image

image

image

About freeCodeCamp/freeCodeCamp#49045, your fixes might just partially address the issue detailed there. From the comments it looks like some people might have had the - operator replaced by 0.

I'm still able to reproduce the specific issue there, where +0+0+0+0+0+0+-1 = -1 is displayed as --1 = -1. Though strangely it only happens if +0 appears 6 or more times. Less than 6 times and everything is displayed properly.

I think we can accept this as an improvement for people running Chrome / Chromium on Macs, and leave that issue open in case someone comes up with a fix.

@Dave2188
Copy link
Contributor Author

I found the fix to the last issue discussed above and another around when you minus a number by a negative number. Should push it to this branch or create a new one for checking?

@Dave2188
Copy link
Contributor Author

Dave2188 commented Feb 17, 2023

@scissorsneedfoodtoo These are some shots of the output after entering +0+0+0+0+0+0+0+-1, which now displays correctly, and 9 - -9 before displayed 18 and now displays correctly with 0.

Screenshot 2023-02-15 at 10 49 59 PM

Screenshot 2023-02-15 at 11 21 23 PM

Screenshot 2023-02-16 at 7 13 06 PM

Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

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

Thank you for all your work on this 👍

@naomi-lgbt naomi-lgbt merged commit 3ab3322 into freeCodeCamp:main Feb 18, 2023
@naomi-lgbt
Copy link
Member

Changes deployed!

@Dave2188 Dave2188 deleted the fcc-calculator-isssue-#49045 branch February 19, 2023 02:01
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.

4 participants