Added counter app in javascript challenges - #536
Conversation
✅ Deploy Preview for frontend-mini-challenges ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughA new Counter App feature is introduced with three files: an HTML page displaying a counter with increment and decrement buttons, JavaScript implementing state management and button functionality, and CSS styling for the interface layout and button appearance. Total 49 lines added across three new files. Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.4.0)apps/javascript/counter-app/style.cssConfigurationError: Could not find "stylelint-config-recess-order". Do you need to install the package or use the "configBasedir" option? 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 Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/javascript/counter-app/index.html (1)
4-7: Add viewport metadata for better mobile rendering.This keeps scaling predictable on mobile devices.
Suggested fix
<head> <meta charset="UTF-8" /> + <meta name="viewport" content="width=device-width, initial-scale=1.0" /> <title>Counter App</title> <link rel="stylesheet" href="style.css" /> </head>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/javascript/counter-app/index.html` around lines 4 - 7, Add a responsive viewport meta tag to index.html to ensure predictable mobile scaling by inserting a meta element like the standard viewport declaration (<meta name="viewport" content="width=device-width, initial-scale=1" />) into the head alongside the existing <meta charset="UTF-8" /> and before the <title> or <link rel="stylesheet" href="style.css" />; update the head content in the file so mobile devices render/layout consistently.apps/javascript/counter-app/style.css (1)
13-19: Add explicit keyboard focus styling for buttons.A dedicated
:focus-visiblestyle improves accessibility consistency.Suggested fix
button { padding: 10px 20px; margin: 5px; font-size: 18px; border-radius: 8px; cursor: pointer; } + +button:focus-visible { + outline: 3px solid `#1d4ed8`; + outline-offset: 2px; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/javascript/counter-app/style.css` around lines 13 - 19, Add explicit keyboard focus styling for interactive buttons by extending the existing button rules: add a :focus-visible selector for button to apply a clear, high-contrast outline or visible box-shadow (e.g., 2-3px solid or offset shadow) and sufficient focus ring color, plus a :focus fallback for browsers without :focus-visible; keep cursor and existing styles and avoid changing layout by using outline-offset or box-shadow rather than changing border or padding. Target the existing button selector and add button:focus-visible (and button:focus fallback) to improve keyboard accessibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/javascript/counter-app/index.html`:
- Around line 11-14: The count display (element with id "count") and the
symbol-only buttons need accessibility updates: add an appropriate live region
attribute (e.g., aria-live="polite" and role="status" or aria-atomic="true") to
the "count" element so screen readers announce changes when
increment()/decrement() updates it, and give the "+" and "−" buttons explicit
accessible names (e.g., aria-label="Increment count" and aria-label="Decrement
count" or associate hidden text via aria-labelledby) so the increment() and
decrement() buttons are announced meaningfully by assistive tech.
In `@apps/javascript/counter-app/script.js`:
- Around line 3-11: Both event handlers (increment() and decrement()) duplicate
DOM lookup and will throw if document.getElementById('count') is null; refactor
by obtaining the element once into a local variable (e.g., const countEl =
document.getElementById('count')) inside each function (or higher scope), check
that countEl exists before mutating, and then update its innerText after
adjusting the shared count variable; replace repeated
document.getElementById('count') calls in increment() and decrement() with this
guarded reference to avoid duplication and runtime errors.
In `@apps/javascript/counter-app/style.css`:
- Around line 1-7: The body rule uses the static viewport unit height:100vh
which can mis-size on mobile; update the body selector (body { ... }) to use
dynamic viewport units—replace height: 100vh with min-height: 100dvh (or height:
100dvh) and keep height: 100vh as a fallback above it for older browsers so
centering remains correct across mobile browser chrome changes.
---
Nitpick comments:
In `@apps/javascript/counter-app/index.html`:
- Around line 4-7: Add a responsive viewport meta tag to index.html to ensure
predictable mobile scaling by inserting a meta element like the standard
viewport declaration (<meta name="viewport" content="width=device-width,
initial-scale=1" />) into the head alongside the existing <meta charset="UTF-8"
/> and before the <title> or <link rel="stylesheet" href="style.css" />; update
the head content in the file so mobile devices render/layout consistently.
In `@apps/javascript/counter-app/style.css`:
- Around line 13-19: Add explicit keyboard focus styling for interactive buttons
by extending the existing button rules: add a :focus-visible selector for button
to apply a clear, high-contrast outline or visible box-shadow (e.g., 2-3px solid
or offset shadow) and sufficient focus ring color, plus a :focus fallback for
browsers without :focus-visible; keep cursor and existing styles and avoid
changing layout by using outline-offset or box-shadow rather than changing
border or padding. Target the existing button selector and add
button:focus-visible (and button:focus fallback) to improve keyboard
accessibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fec28bfb-7611-4542-bccb-50a3813239e6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
apps/javascript/counter-app/index.htmlapps/javascript/counter-app/script.jsapps/javascript/counter-app/style.css
| <h2 id="count">0</h2> | ||
|
|
||
| <button onclick="increment()">+</button> | ||
| <button onclick="decrement()">-</button> |
There was a problem hiding this comment.
Improve accessibility for dynamic count and symbol-only buttons.
Add live-region semantics to the counter and explicit labels for +/- buttons.
Suggested fix
- <h2 id="count">0</h2>
+ <h2 id="count" role="status" aria-live="polite" aria-atomic="true">0</h2>
- <button onclick="increment()">+</button>
- <button onclick="decrement()">-</button>
+ <button onclick="increment()" aria-label="Increment counter">+</button>
+ <button onclick="decrement()" aria-label="Decrement counter">-</button>📝 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.
| <h2 id="count">0</h2> | |
| <button onclick="increment()">+</button> | |
| <button onclick="decrement()">-</button> | |
| <h2 id="count" role="status" aria-live="polite" aria-atomic="true">0</h2> | |
| <button onclick="increment()" aria-label="Increment counter">+</button> | |
| <button onclick="decrement()" aria-label="Decrement counter">-</button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/javascript/counter-app/index.html` around lines 11 - 14, The count
display (element with id "count") and the symbol-only buttons need accessibility
updates: add an appropriate live region attribute (e.g., aria-live="polite" and
role="status" or aria-atomic="true") to the "count" element so screen readers
announce changes when increment()/decrement() updates it, and give the "+" and
"−" buttons explicit accessible names (e.g., aria-label="Increment count" and
aria-label="Decrement count" or associate hidden text via aria-labelledby) so
the increment() and decrement() buttons are announced meaningfully by assistive
tech.
| function increment() { | ||
| count++; | ||
| document.getElementById('count').innerText = count; | ||
| } | ||
|
|
||
| function decrement() { | ||
| count--; | ||
| document.getElementById('count').innerText = count; | ||
| } |
There was a problem hiding this comment.
Avoid repeated DOM queries and guard against missing #count.
Both handlers duplicate update logic and can throw if #count is not found.
Suggested fix
let count = 0;
+const countElement = document.getElementById('count');
+
+function renderCount() {
+ if (!countElement) return;
+ countElement.innerText = String(count);
+}
function increment() {
count++;
- document.getElementById('count').innerText = count;
+ renderCount();
}
function decrement() {
count--;
- document.getElementById('count').innerText = count;
+ renderCount();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/javascript/counter-app/script.js` around lines 3 - 11, Both event
handlers (increment() and decrement()) duplicate DOM lookup and will throw if
document.getElementById('count') is null; refactor by obtaining the element once
into a local variable (e.g., const countEl = document.getElementById('count'))
inside each function (or higher scope), check that countEl exists before
mutating, and then update its innerText after adjusting the shared count
variable; replace repeated document.getElementById('count') calls in increment()
and decrement() with this guarded reference to avoid duplication and runtime
errors.
| body { | ||
| font-family: Arial; | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| height: 100vh; | ||
| } |
There was a problem hiding this comment.
Use dynamic viewport height to avoid mobile centering glitches.
height: 100vh can mis-size on mobile when browser chrome expands/collapses. Prefer dynamic viewport units.
Suggested fix
body {
font-family: Arial;
display: flex;
justify-content: center;
align-items: center;
- height: 100vh;
+ min-height: 100dvh;
+ margin: 0;
}📝 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.
| body { | |
| font-family: Arial; | |
| display: flex; | |
| justify-content: center; | |
| align-items: center; | |
| height: 100vh; | |
| } | |
| body { | |
| font-family: Arial; | |
| display: flex; | |
| justify-content: center; | |
| align-items: center; | |
| min-height: 100dvh; | |
| margin: 0; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/javascript/counter-app/style.css` around lines 1 - 7, The body rule uses
the static viewport unit height:100vh which can mis-size on mobile; update the
body selector (body { ... }) to use dynamic viewport units—replace height: 100vh
with min-height: 100dvh (or height: 100dvh) and keep height: 100vh as a fallback
above it for older browsers so centering remains correct across mobile browser
chrome changes.
|
We already have the counter app. Is this different |
Added a simple counter app using HTML, CSS, and JavaScript.
Features:
Summary by CodeRabbit