-
-
Notifications
You must be signed in to change notification settings - Fork 83
add log button #181
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
base: main
Are you sure you want to change the base?
add log button #181
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -173,6 +173,8 @@ <h1 id="calc-style">Calculator<i class="bx bx-calculator"></i></h1> | |||||
| <button class="button complex-stuff"> | ||||||
| tanh | ||||||
| </button> | ||||||
| <button class="button" id="logBtn"> | ||||||
| log</button> | ||||||
|
Comment on lines
+176
to
+177
|
||||||
| <button class="button" id="logBtn"> | |
| log</button> |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -245,6 +245,17 @@ Array.prototype.forEach.call(buttons, function (button) { | |||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| document.getElementById('logBtn').addEventListener('click', log); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
245
to
+248
|
||||||||||||||||||||||||||||||||||||||
| } | |
| }); | |
| }); | |
| document.getElementById('logBtn').addEventListener('click', log); | |
| } else if (trimmedButtonValue === 'log') { | |
| log(); | |
| } | |
| }); | |
| }); |
Copilot
AI
Feb 21, 2026
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.
The new log button bypasses the established button handling pattern. All other calculator buttons with the "complex-stuff" class are handled through the main Array.prototype.forEach.call event listener (lines 104-247). Using getElementById with a separate event listener creates inconsistency. Additionally, when this button is clicked, it will trigger both this handler AND the main button handler at line 104, potentially causing the function to execute twice or behave unexpectedly.
| } | |
| }); | |
| }); | |
| document.getElementById('logBtn').addEventListener('click', log); | |
| } else if (trimmedButtonValue === 'log') { | |
| log(); | |
| } | |
| }); | |
| }); |
Copilot
AI
Feb 21, 2026
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.
While the error handling for non-positive numbers is a good addition, it creates inconsistency with the existing log10() function (line 515-518) which performs the same operation but lacks this validation. When users try log₁₀ of a negative number, they get NaN displayed, but with this new 'log' button they would get an error toast. This inconsistent behavior for the same mathematical operation could confuse users. The validation logic should either be added to all logarithm functions (log10, log2, loge) or removed from this one to maintain consistency.
| if (isNaN(num) || num <= 0) { | |
| openToast('error', 'Error', 'Enter a positive number to calculate log'); | |
| return; | |
| } |
Copilot
AI
Feb 21, 2026
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.
This function duplicates the existing log10() function at line 515-518, which already calculates the base-10 logarithm using Math.log10(). The existing implementation also handles the same operation and follows the established pattern of using eval() for consistency with other math functions. Having two functions that do the same thing creates maintenance issues and confusion.
| document.getElementById('logBtn').addEventListener('click', log); | |
| function log() { | |
| const num = parseFloat(display.value); | |
| if (isNaN(num) || num <= 0) { | |
| openToast('error', 'Error', 'Enter a positive number to calculate log'); | |
| return; | |
| } | |
| display.value = Math.log10(num); | |
| resultDisplayed = true; | |
| manageLocalStorage(display.value); | |
| } | |
| document.getElementById('logBtn').addEventListener('click', log10); |
Copilot
AI
Feb 21, 2026
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.
The play() function should be called to provide audio feedback, consistent with all other calculator operations. See line 106 in the main button handler and line 74 in error handling for examples. The audio feedback is an important UX element that should be maintained across all operations.
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.
The button is missing the "complex-stuff" class that all other buttons in the "more functions" section have (see lines 94-175). This class is used for styling and potentially for the button event handling system. The inconsistent class assignment may cause visual inconsistencies or prevent the button from being properly handled by the main event listener loop.