Implemented feature f01: Bin2Dec converter#1111
Open
asthanit1205 wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe PR introduces a new Binary to Decimal Converter web application with a complete feature set: HTML UI structure featuring a form with binary input validation, JavaScript logic for input sanitization, validation, and conversion, and a responsive CSS stylesheet with modern styling including glassmorphism effects and accessibility support. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
bin2dec-app/style.css (1)
42-43: Add reduced-motion support for animation and hover motion.The current motion effects have no
prefers-reduced-motionfallback. Please disable or reduce them for users who opt out of animations.Suggested change
`@media` (max-width: 640px) { .card { padding: 22px; border-radius: 18px; } } + +@media (prefers-reduced-motion: reduce) { + .card { + animation: none; + } + + button { + transition: none; + } + + button:hover { + transform: none; + } +}Also applies to: 113-119, 141-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin2dec-app/style.css` around lines 42 - 43, The CSS animations (e.g., the rule that sets "animation: rise-in 420ms ease-out;") and the hover motion blocks at the ranges noted need a prefers-reduced-motion fallback: wrap alternative styles in a `@media` (prefers-reduced-motion: reduce) query that either removes animations/transitions (set animation: none and transition: none) or reduces durations to near-zero for the same selectors that declare "animation: rise-in..." and the hover rules referenced (lines ~113-119 and ~141-150); update the same selectors (and any :hover/:focus motion rules) so users who prefer reduced motion get no/very-short animations while keeping the original animations outside that media query.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin2dec-app/index.html`:
- Line 41: The element with id "error-text" currently mixes role="alert" and
aria-live="polite", causing inconsistent SR behavior; update the <p
id="error-text"> element (identifier: error-text) to use a single live-region
strategy by removing role="alert" and keeping aria-live="polite" (or vice versa
if assertive is required), and ensure the element still has appropriate aria
attributes (e.g., aria-live="polite") for inline validation announcements.
In `@bin2dec-app/script.js`:
- Around line 33-47: The validateInput function currently enforces only the
8-digit limit; update it to also enforce a 64-digit cap when bonus mode is
enabled by checking allowLongInput and returning an appropriate error if
binaryValue.length > 64. Keep existing checks (empty input and isBinaryString)
and ensure the error messages differentiate the two limits (e.g., "Use up to 8
digits..." vs "Use up to 64 digits, or disable bonus mode."). Modify
validateInput (and reference allowLongInput, binaryValue, isBinaryString) to run
the 64-length check before returning success.
- Around line 19-31: The binaryToDecimal function should use BigInt to avoid
Number precision loss: change decimalValue to BigInt(0), use BigInt(2)
exponentiation or bit shifts and add BigInt(1) when a digit is "1" (e.g.,
shift/<< with BigInt), and keep position tracking as BigInt arithmetic; also
update the code that assigns/displays the conversion result (the result
variable/assignment where the function is called) to convert the BigInt to a
string (via toString()) before rendering. Ensure all intermediate literals and
arithmetic in binaryToDecimal use BigInt types and that the returned value is a
BigInt so the caller transforms it to a string for display.
---
Nitpick comments:
In `@bin2dec-app/style.css`:
- Around line 42-43: The CSS animations (e.g., the rule that sets "animation:
rise-in 420ms ease-out;") and the hover motion blocks at the ranges noted need a
prefers-reduced-motion fallback: wrap alternative styles in a `@media`
(prefers-reduced-motion: reduce) query that either removes
animations/transitions (set animation: none and transition: none) or reduces
durations to near-zero for the same selectors that declare "animation:
rise-in..." and the hover rules referenced (lines ~113-119 and ~141-150); update
the same selectors (and any :hover/:focus motion rules) so users who prefer
reduced motion get no/very-short animations while keeping the original
animations outside that media query.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aef5492c-1e13-4200-aae4-f77694e10128
📒 Files selected for processing (3)
bin2dec-app/index.htmlbin2dec-app/script.jsbin2dec-app/style.css
| <button type="submit">Convert</button> | ||
| </form> | ||
|
|
||
| <p id="error-text" class="error" role="alert" aria-live="polite"></p> |
There was a problem hiding this comment.
Use a single live-region strategy for error announcements.
Line 41 mixes role="alert" (assertive) with aria-live="polite". That can lead to inconsistent screen-reader behavior. Keep one (typically aria-live="polite" for inline validation).
Suggested change
- <p id="error-text" class="error" role="alert" aria-live="polite"></p>
+ <p id="error-text" class="error" aria-live="polite"></p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <p id="error-text" class="error" role="alert" aria-live="polite"></p> | |
| <p id="error-text" class="error" aria-live="polite"></p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin2dec-app/index.html` at line 41, The element with id "error-text"
currently mixes role="alert" and aria-live="polite", causing inconsistent SR
behavior; update the <p id="error-text"> element (identifier: error-text) to use
a single live-region strategy by removing role="alert" and keeping
aria-live="polite" (or vice versa if assertive is required), and ensure the
element still has appropriate aria attributes (e.g., aria-live="polite") for
inline validation announcements.
Comment on lines
+19
to
+31
| function binaryToDecimal(binaryValue) { | ||
| let decimalValue = 0; | ||
| let positionFromRight = 0; | ||
|
|
||
| for (let i = binaryValue.length - 1; i >= 0; i -= 1) { | ||
| if (binaryValue[i] === "1") { | ||
| decimalValue += Math.pow(2, positionFromRight); | ||
| } | ||
| positionFromRight += 1; | ||
| } | ||
|
|
||
| return decimalValue; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: demonstrate precision loss in Number-based approach.
node - <<'NODE'
const binary = '1'.repeat(64);
let dec = 0;
let pos = 0;
for (let i = binary.length - 1; i >= 0; i--) {
if (binary[i] === '1') dec += Math.pow(2, pos);
pos++;
}
const expected = (2n ** 64n) - 1n;
console.log('number_result:', dec);
console.log('number_as_bigint:', BigInt(dec).toString());
console.log('expected:', expected.toString());
console.log('exact_match:', BigInt(dec) === expected);
NODERepository: florinpop17/app-ideas
Length of output: 189
🏁 Script executed:
#!/bin/bash
# Verify the actual code in the repository and check related lines
# First, let's find and read the script.js file
fd "script\.js" bin2dec-app -type f
# Then check the structure around lines 19-31 and 67-68
if [ -f "bin2dec-app/script.js" ]; then
echo "=== Lines 15-75 of script.js ==="
cat -n bin2dec-app/script.js | sed -n '15,75p'
fiRepository: florinpop17/app-ideas
Length of output: 1959
64-bit bonus inputs produce incorrect decimals due to Number precision limits.
The binaryToDecimal function accumulates results into a Number type, which cannot exactly represent integers beyond 2^53-1. For 64-bit binary inputs (up to 2^64-1), this causes precision loss. For example, the maximum 64-bit value (all ones) converts to 18446744073709552000 instead of the correct 18446744073709551615.
Use BigInt for exact conversion. This affects both the function at lines 19-31 and the result assignment at lines 67-68.
Suggested fix (use BigInt for exact conversion)
function binaryToDecimal(binaryValue) {
- let decimalValue = 0;
+ let decimalValue = 0n;
let positionFromRight = 0;
for (let i = binaryValue.length - 1; i >= 0; i -= 1) {
if (binaryValue[i] === "1") {
- decimalValue += Math.pow(2, positionFromRight);
+ decimalValue += 1n << BigInt(positionFromRight);
}
positionFromRight += 1;
}
return decimalValue;
}
@@
- const result = binaryToDecimal(binaryValue);
- decimalOutput.textContent = String(result);
+ const result = binaryToDecimal(binaryValue);
+ decimalOutput.textContent = result.toString();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin2dec-app/script.js` around lines 19 - 31, The binaryToDecimal function
should use BigInt to avoid Number precision loss: change decimalValue to
BigInt(0), use BigInt(2) exponentiation or bit shifts and add BigInt(1) when a
digit is "1" (e.g., shift/<< with BigInt), and keep position tracking as BigInt
arithmetic; also update the code that assigns/displays the conversion result
(the result variable/assignment where the function is called) to convert the
BigInt to a string (via toString()) before rendering. Ensure all intermediate
literals and arithmetic in binaryToDecimal use BigInt types and that the
returned value is a BigInt so the caller transforms it to a string for display.
Comment on lines
+33
to
+47
| function validateInput(binaryValue, allowLongInput) { | ||
| if (!binaryValue) { | ||
| return "Please enter a binary number."; | ||
| } | ||
|
|
||
| if (!isBinaryString(binaryValue)) { | ||
| return "Only 0 and 1 are allowed."; | ||
| } | ||
|
|
||
| if (!allowLongInput && binaryValue.length > 8) { | ||
| return "Use up to 8 digits, or enable bonus mode."; | ||
| } | ||
|
|
||
| return ""; | ||
| } |
There was a problem hiding this comment.
Mirror bonus-mode length limit in validation logic.
Validation currently enforces only the non-bonus 8-digit rule. Add an explicit 64-digit check so the business rule is not only UI-attribute-dependent.
Suggested change
function validateInput(binaryValue, allowLongInput) {
if (!binaryValue) {
return "Please enter a binary number.";
}
@@
if (!allowLongInput && binaryValue.length > 8) {
return "Use up to 8 digits, or enable bonus mode.";
}
+
+ if (allowLongInput && binaryValue.length > 64) {
+ return "Use up to 64 digits in bonus mode.";
+ }
return "";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function validateInput(binaryValue, allowLongInput) { | |
| if (!binaryValue) { | |
| return "Please enter a binary number."; | |
| } | |
| if (!isBinaryString(binaryValue)) { | |
| return "Only 0 and 1 are allowed."; | |
| } | |
| if (!allowLongInput && binaryValue.length > 8) { | |
| return "Use up to 8 digits, or enable bonus mode."; | |
| } | |
| return ""; | |
| } | |
| function validateInput(binaryValue, allowLongInput) { | |
| if (!binaryValue) { | |
| return "Please enter a binary number."; | |
| } | |
| if (!isBinaryString(binaryValue)) { | |
| return "Only 0 and 1 are allowed."; | |
| } | |
| if (!allowLongInput && binaryValue.length > 8) { | |
| return "Use up to 8 digits, or enable bonus mode."; | |
| } | |
| if (allowLongInput && binaryValue.length > 64) { | |
| return "Use up to 64 digits in bonus mode."; | |
| } | |
| return ""; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin2dec-app/script.js` around lines 33 - 47, The validateInput function
currently enforces only the 8-digit limit; update it to also enforce a 64-digit
cap when bonus mode is enabled by checking allowLongInput and returning an
appropriate error if binaryValue.length > 64. Keep existing checks (empty input
and isBinaryString) and ensure the error messages differentiate the two limits
(e.g., "Use up to 8 digits..." vs "Use up to 64 digits, or disable bonus
mode."). Modify validateInput (and reference allowLongInput, binaryValue,
isBinaryString) to run the 64-length check before returning success.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feature implemented:
f01 Bin2Dec Converter
Changes made:
Added binary input with validation (0/1 only)
Added decimal output field
Added conversion logic using power-of-2 method
Added optional variable-length bonus mode
Files modified:
index.html
style.css
script.js
Output description:
Accepts binary inputShows validation errors for invalid characters
Displays decimal equivalent
Screenshots:
Add at least 2:
Valid input example (like 10110101 -> 181)
Invalid input example (like 10201 shows error)
Summary by CodeRabbit
New Features