Added Example Dropdown with Multiple Sample Codes #29
Conversation
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Editor & Language setupkidcode-web/src/main/resources/static/app.js |
Reworked Monaco loading/init; registered language id "kidcode" with a flattened Monarch tokenizer; consolidated editor creation and actions; added initializeExamples with guards; debounced validateCode (POST /api/validate) mapping errors to Monaco markers; kept /api/execute run flow; added canvas export and standardized helpers/events. |
Examples dataset (new)kidcode-web/src/main/resources/static/examples.js |
New file exposing window.examples: categorized easy/intermediate/advanced multi-line KidCode examples for the selector. |
UI markupkidcode-web/src/main/resources/static/index.html |
Added #exampleSelector and wrapper, replaced editor placeholder with #editor-container, moved controls into .panel-controls, restructured Help modal, added examples.js script, updated stylesheet query param. |
Styling / Header layoutkidcode-web/src/main/resources/static/style.css |
New header/panel styles: .panel-header, .panel-left, .panel-controls, .example-selector-wrapper, .sr-only, #exampleSelector styles; spacing and button layout tweaks; minor formatting edits. |
Sequence Diagram(s)
sequenceDiagram
autonumber
actor User
participant Selector as ExampleSelector
participant App as app.js
participant Editor as Monaco Editor
User->>Selector: select example
Selector->>App: change event
App->>App: lookup window.examples (guard)
App->>Editor: editor.setValue(code)
Editor-->>User: code updated in UI
sequenceDiagram
autonumber
actor User
participant Editor as Monaco Editor
participant App as app.js
participant Validate as /api/validate
participant Execute as /api/execute
participant Canvas as Renderer
rect rgba(230,240,255,0.35)
Editor->>App: onDidChangeModelContent (debounced)
App->>Validate: POST /api/validate { code }
Validate-->>App: validation result (errors/ok)
App->>Editor: setModelMarkers(errors)
end
rect rgba(240,255,230,0.35)
User->>App: Run (button or Ctrl/Cmd+Enter)
App->>Execute: POST /api/execute { code }
Execute-->>App: execution events / response
loop per event
App->>Canvas: renderEvents(event)
end
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
I twitch my whiskers, nibble keys,
Examples scatter like spring trees,
Monaco hums, the markers shine,
Cody draws a looping line,
Hop—press run, and watch it be! 🐇✨
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title Check | ✅ Passed | The title succinctly describes the primary change—adding an example dropdown populated with multiple sample code scripts—and aligns with modifications in index.html, examples.js, and app.js. It is concise, clear, and specific enough for team members to understand the main feature at a glance. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
📜 Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
kidcode-web/src/main/resources/static/app.js(1 hunks)
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
kidcode-web/src/main/resources/static/examples.js (1)
3-3: Consider using a namespaced or module-based approach.Using
window.examplesdirectly pollutes the global namespace. For better maintainability and to avoid potential naming conflicts, consider one of these approaches:
- Option 1: Use a namespaced global like
window.KidCode = { examples: {...} }- Option 2: Convert to an ES6 module and export the examples
- Option 3: Use an IIFE to minimize global exposure
Example using Option 1:
-window.examples = { +window.KidCode = window.KidCode || {}; +window.KidCode.examples = { /* 🟢 EASY LEVEL (Basics) */Then update
app.jsto referencewindow.KidCode.examplesinstead ofwindow.examples.kidcode-web/src/main/resources/static/app.js (2)
116-122: Redundant check for window.examples.Line 118 checks
window.examplesexistence again, but ifexamples.jsdidn't load, the dropdown wouldn't be populated (assuming the previous check is added). The check for the selected key is good, but consider simplifying after adding the initialization check.After adding the safety check during initialization, you can simplify this to:
selector.addEventListener("change", () => { const selected = selector.value; - if (window.examples && window.examples[selected]) { + if (selected && window.examples[selected]) { editor.setValue(window.examples[selected]); logToOutput(`✅ Loaded example: ${selected}`); } });
165-186: Validation error handling could be more robust.The
validateCodefunction silently swallows validation errors (line 184) without providing user feedback. While logging to console is helpful for debugging, users won't know if validation failed.Consider providing user feedback when validation fails:
} catch (error) { console.error("Validation request failed:", error); + logToOutput(`⚠️ Code validation unavailable: ${error.message}`, "error"); + // Clear existing markers since we can't validate + monaco.editor.setModelMarkers(editor.getModel(), "kidcode", []); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
kidcode-web/src/main/resources/static/app.js(1 hunks)kidcode-web/src/main/resources/static/examples.js(1 hunks)kidcode-web/src/main/resources/static/index.html(1 hunks)kidcode-web/src/main/resources/static/style.css(1 hunks)
🔇 Additional comments (6)
kidcode-web/src/main/resources/static/style.css (1)
209-217: LGTM! Clean CSS addition.The styling for
#exampleSelectoris appropriate, and the missing closing brace for#help-docs thhas been correctly added.kidcode-web/src/main/resources/static/examples.js (1)
6-165: Well-structured example collection!The examples are well-organized with clear progression from easy to advanced. They cover the key language features effectively and provide good learning material for beginners.
kidcode-web/src/main/resources/static/index.html (2)
26-28: Clean selector implementation.The example selector is appropriately positioned and structured. The empty value option provides a good default state.
226-227: Script loading order is correct.The ordering ensures
examples.jsloads beforeapp.js, which is necessary sinceapp.jsreferenceswindow.examplesat initialization time.kidcode-web/src/main/resources/static/app.js (2)
124-133: Nice UX enhancement with the keyboard shortcut!The Ctrl/Cmd+Enter keybinding provides a convenient way to run code without clicking, which is a common expectation in code editors.
72-104: Default example is well-chosen.The rainbow spiral provides an engaging and visually appealing introduction that demonstrates multiple language features (variables, arrays, loops, conditionals). Good choice for first-time users!
|
Added defensive check for window.examples as suggested. Ready for review. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kidcode-web/src/main/resources/static/index.html(2 hunks)kidcode-web/src/main/resources/static/style.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- kidcode-web/src/main/resources/static/style.css
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
kidcode-web/src/main/resources/static/style.css (1)
99-109: Consolidate duplicate.panel-controlsdeclarations.The
.panel-controlsclass is declared twice (lines 99-103 and 106-109) with different properties. Merge them into a single rule block for clarity.Apply this diff:
-/* 🖲️ Right side: Buttons stay aligned */ -.panel-controls { - display: flex; - align-items: center; - gap: 14px; -} - -/* Ensure controls don't wrap and keep their size */ .panel-controls { + display: flex; + align-items: center; + gap: 14px; flex-shrink: 0; white-space: nowrap; }kidcode-web/src/main/resources/static/app.js (1)
128-128: Remove redundant window.examples check.Line 128 checks
window.examplesagain, but this is redundant since you already verified it exists in the check above (line 109).Apply this diff:
selector.addEventListener("change", () => { const selected = selector.value; - if (window.examples && window.examples[selected]) { + if (window.examples[selected]) { editor.setValue(window.examples[selected]); logToOutput(`✅ Loaded example: ${selected}`); } });Note: This suggestion applies to the current code structure. If you refactor per the previous comment, this check should be adjusted accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
kidcode-web/src/main/resources/static/app.js(1 hunks)kidcode-web/src/main/resources/static/index.html(1 hunks)kidcode-web/src/main/resources/static/style.css(3 hunks)
🔇 Additional comments (7)
kidcode-web/src/main/resources/static/style.css (2)
37-97: Well-structured panel header and dropdown styling.The panel header layout with flex positioning, dropdown styling, and hover states are well-implemented and maintain visual consistency.
112-125: Proper sr-only implementation for accessibility.The visually-hidden utility follows WCAG best practices, ensuring content remains available to screen readers while being hidden visually.
kidcode-web/src/main/resources/static/index.html (2)
29-31: Accessibility concern addressed.The
aria-label="Choose Example"attribute successfully addresses the previous review comment about accessible labeling for the example selector. While a visible<label>element would be ideal per WCAG guidelines, thearia-labelis acceptable given the compact header layout where the dropdown appears adjacent to the "Code Editor" heading.
24-54: Well-structured editor panel layout.The new panel structure cleanly separates the header controls (title, dropdown, buttons) from the editor container, improving maintainability and visual organization.
kidcode-web/src/main/resources/static/app.js (3)
134-143: Excellent editor action for Run command.The keyboard shortcut (Ctrl/Cmd+Enter) to trigger the Run button provides good UX and follows common IDE conventions. The implementation is clean and straightforward.
175-196: Solid live validation implementation.The validation logic correctly posts to
/api/validate, maps server errors to Monaco markers, and handles errors gracefully. The debouncing viavalidationTimeoutprevents excessive API calls.
239-277: Well-structured event rendering pipeline.The event processing loop correctly handles different event types (Clear, Move, Say, Error), maintains drawing state, and efficiently redraws the canvas only after all events are processed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
kidcode-web/src/main/resources/static/app.js (4)
182-203: Avoid stale validation markers; abort in-flight requests and validate response.ok.Concurrent keystrokes can race; older responses can override newer markers. Also handle non-OK responses and compute precise endColumn.
Apply this diff to harden validateCode:
async function validateCode() { - const code = editor.getValue(); - try { - const response = await fetch("/api/validate", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ code: code }), - }); - const errors = await response.json(); - const markers = errors.map((err) => ({ - message: err.message, - severity: monaco.MarkerSeverity.Error, - startLineNumber: err.lineNumber, - endLineNumber: err.lineNumber, - startColumn: 1, - endColumn: 100, - })); - monaco.editor.setModelMarkers(editor.getModel(), "kidcode", markers); - } catch (error) { - console.error("Validation request failed:", error); - } + const code = editor.getValue(); + + // Cancel any in-flight request to avoid race conditions + if (validationController) { + validationController.abort(); + } + const controller = new AbortController(); + validationController = controller; + + try { + const response = await fetch("/api/validate", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ code }), + signal: controller.signal, + }); + if (!response.ok) { + throw new Error(`HTTP error ${response.status}`); + } + const errors = await response.json(); + const model = editor.getModel(); + const markers = (Array.isArray(errors) ? errors : []).map((err) => { + const line = Number(err.lineNumber) > 0 ? Number(err.lineNumber) : 1; + const endCol = model ? model.getLineMaxColumn(line) : 100; + return { + message: String(err.message || "Syntax error"), + severity: monaco.MarkerSeverity.Error, + startLineNumber: line, + endLineNumber: line, + startColumn: 1, + endColumn: endCol, + }; + }); + monaco.editor.setModelMarkers(model, "kidcode", markers); + } catch (error) { + if (error.name === "AbortError") return; // superseded by a newer request + console.error("Validation request failed:", error); + } finally { + if (validationController === controller) validationController = null; + } }Also add a controller at the top-level:
let editor; -let validationTimeout; +let validationTimeout; +let validationController; // used to abort stale validations
160-179: Prevent stale drawings across runs (if backend omits ClearEvent).If the server doesn’t send a ClearEvent, old lines reappear after redraw. Reset local state on Run or confirm backend always emits ClearEvent at start.
runButton.addEventListener("click", async () => { const code = editor.getValue(); + // Ensure a clean slate even if backend omits ClearEvent + drawnLines = []; + codyState = { x: 250, y: 250, direction: 0, color: "blue" }; clearCanvas(); outputArea.textContent = "";Would you prefer relying on backend ClearEvent instead? If yes, confirm it’s guaranteed so we can skip this reset.
51-60: Tokenizer nit: support decimals/negatives and non-invalid escapes.Current tokens only match integers and treat all escapes as invalid. If KidCode allows decimals/negatives and common escapes, consider:
- string: [ - [/[^\\"]+/, "string"], - [/\\./, "string.escape.invalid"], - [/"/, { token: "string.quote", bracket: "@close", next: "@pop" }], - ], + string: [ + [/[^\\"]+/, "string"], + [/\\./, "string.escape"], // accept generic escapes + [/"/, { token: "string.quote", bracket: "@close", next: "@pop" }], + ],And for numbers:
- [/\d+/, "number"], + [/-?\d+(?:\.\d+)?/, "number"], // optional: decimals and negativesAdjust based on the language spec.
249-274: Harden rendering when event.color is missing.Fallback to current Cody color to avoid undefined strokeStyle on partial events.
case "MoveEvent": if ( event.isPenDown && (event.fromX !== event.toX || event.fromY !== event.toY) ) { drawnLines.push({ fromX: event.fromX, fromY: event.fromY, toX: event.toX, toY: event.toY, - color: event.color, + color: event.color || codyState.color, }); } codyState = { x: event.toX, y: event.toY, direction: event.newDirection, - color: event.color, + color: event.color ?? codyState.color, }; break;Confirm if the backend always sets color; if yes, you can skip this.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
kidcode-web/src/main/resources/static/app.js(1 hunks)
🔇 Additional comments (2)
kidcode-web/src/main/resources/static/app.js (2)
107-109: Non-blocking example init looks good.Extracting example setup into initializeExamples() and calling it post-editor init resolves the earlier early-return issue and degrades gracefully when examples.js is missing. Nice.
Also applies to: 129-158
4-11: Verify script load order/DOM readiness.Top-level element lookups and getContext depend on DOM being ready and loader being present. Ensure index.html loads loader.js (Monaco AMD) and places app.js after DOM or with defer to avoid null derefs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
kidcode-web/src/main/resources/static/style.css (3)
40-48: Prevent header overflow on small screens (responsive wrap).
.panel-headerand.panel-controlsforce nowrap and panels havemin-width: 400px, which can overflow on narrow viewports. Add a small-screen override to allow wrapping and avoid horizontal scroll.Add this at the end of the file:
@media (max-width: 640px) { .panel-header { flex-wrap: wrap; gap: 8px; } .panel-left { flex-wrap: wrap; min-width: 0; } .panel-controls { flex-wrap: wrap; white-space: normal; width: 100%; } #exampleSelector { max-width: 100%; } }Also applies to: 51-56, 98-104
127-145: Add visible keyboard focus for buttons.Run/Help buttons have hover/active styles but no explicit
:focus-visiblering. Add an accessible, high-contrast focus indicator.Append:
.animated-btn:focus-visible, .run-btn:focus-visible, .help-btn:focus-visible { outline: 3px solid #1b4f72; outline-offset: 3px; box-shadow: 0 0 0 4px rgba(52,152,219,0.25); }Also applies to: 147-187, 189-229
268-271: Respect prefers-reduced-motion.Disable non-essential animations for users who prefer reduced motion.
Append:
@media (prefers-reduced-motion: reduce) { * { animation: none !important; transition: none !important; } }kidcode-web/src/main/resources/static/app.js (4)
330-342: Guard modal event listeners to avoid NPEs when elements are missing.If any of these elements are absent,
addEventListenerwill throw and break init.- helpButton.addEventListener("click", () => { - helpModal.classList.remove("hidden"); - }); + if (helpButton && helpModal) { + helpButton.addEventListener("click", () => { + helpModal.classList.remove("hidden"); + }); + } - closeButton.addEventListener("click", () => { - helpModal.classList.add("hidden"); - }); + if (closeButton && helpModal) { + closeButton.addEventListener("click", () => { + helpModal.classList.add("hidden"); + }); + } - window.addEventListener("click", (event) => { - if (event.target === helpModal) { - helpModal.classList.add("hidden"); - } - }); + if (helpModal) { + window.addEventListener("click", (event) => { + if (event.target === helpModal) { + helpModal.classList.add("hidden"); + } + }); + }
162-187: Disable Run button during execution to prevent double-submits.Prevents concurrent runs and accidental spamming.
-runButton.addEventListener("click", async () => { +runButton.addEventListener("click", async () => { const code = editor.getValue(); // 🧹 Ensure a clean slate even if backend omits ClearEvent drawnLines = []; codyState = { x: 250, y: 250, direction: 0, color: "blue" }; clearCanvas(); outputArea.textContent = ""; - try { + runButton.disabled = true; + try { const response = await fetch("/api/execute", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ code }), }); if (!response.ok) { throw new Error(`HTTP error! status: ${response.status}`); } const events = await response.json(); renderEvents(events); } catch (error) { logToOutput(`Network or server error: ${error.message}`, "error"); + } finally { + runButton.disabled = false; } });
67-69: CDN-only Monaco load: consider SRI and/or local fallback.If CDN is blocked/offline, editor won’t load. Add Subresource Integrity and/or a local fallback path.
Example:
<script src="https://cdn.jsdelivr.net/npm/monaco-editor@0.34.1/min/vs/loader.min.js" integrity="sha384-…your-SRI…" crossorigin="anonymous"></script>And fallback in require.config or script tag to a bundled local
vspath when CDN fails.
263-271: Auto-scroll output to latest message.Keeps the newest log visible during runs.
if (type === "error") { line.style.color = "red"; line.style.fontWeight = "bold"; } outputArea.appendChild(line); + outputArea.scrollTop = outputArea.scrollHeight;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kidcode-web/src/main/resources/static/app.js(1 hunks)kidcode-web/src/main/resources/static/style.css(3 hunks)
🔇 Additional comments (2)
kidcode-web/src/main/resources/static/style.css (1)
107-120: Good accessibility utility (.sr-only).Solid, cross-browser visually-hidden pattern. LGTM.
kidcode-web/src/main/resources/static/app.js (1)
130-159: Verify examples shape; consider for categories.PR mentions Easy/Medium/Advanced groups. If
window.examplesis grouped, render categories with<optgroup>for better UX.Optional pattern:
function initializeExamples() { const selector = document.getElementById("exampleSelector"); if (!selector) return; if (!window.examples) { selector.disabled = true; selector.innerHTML = '<option value="">Examples unavailable</option>'; return; } // If grouped: { Easy: {Name: Code}, Medium: {...}, Advanced: {...} } Object.entries(window.examples).forEach(([group, items]) => { const og = document.createElement("optgroup"); og.label = group; Object.entries(items).forEach(([name, code]) => { const opt = document.createElement("option"); opt.value = name; opt.textContent = name; og.appendChild(opt); }); selector.appendChild(og); }); selector.addEventListener("change", () => { const selected = selector.value; // Find in groups for (const items of Object.values(window.examples)) { if (items[selected]) { editor.setValue(items[selected]); logToOutput(`✅ Loaded example: ${selected}`); break; } } }); }
| const markers = (Array.isArray(errors) ? errors : []).map((err) => { | ||
| const line = Number(err.lineNumber) > 0 ? Number(err.lineNumber) : 1; | ||
| const endCol = model ? model.getLineMaxColumn(line) : 100; | ||
| return { | ||
| message: String(err.message || "Syntax error"), | ||
| severity: monaco.MarkerSeverity.Error, | ||
| startLineNumber: line, | ||
| endLineNumber: line, | ||
| startColumn: 1, | ||
| endColumn: endCol, | ||
| }; | ||
| }); | ||
|
|
||
| monaco.editor.setModelMarkers(model, "kidcode", markers); |
There was a problem hiding this comment.
Clamp marker line numbers to model bounds to avoid runtime errors.
getLineMaxColumn(line) may throw if line > lineCount. Clamp to [1, lineCount] and derive severity if provided.
Apply:
- const markers = (Array.isArray(errors) ? errors : []).map((err) => {
- const line = Number(err.lineNumber) > 0 ? Number(err.lineNumber) : 1;
- const endCol = model ? model.getLineMaxColumn(line) : 100;
+ const markers = (Array.isArray(errors) ? errors : []).map((err) => {
+ const rawLine = Number(err.lineNumber) || 1;
+ const lineCount = model ? model.getLineCount() : 1;
+ const line = Math.max(1, Math.min(rawLine, lineCount));
+ const endCol = model ? model.getLineMaxColumn(line) : 100;
+ const sev =
+ (err.severity === "warning" && monaco.MarkerSeverity.Warning) ||
+ (err.severity === "info" && monaco.MarkerSeverity.Info) ||
+ monaco.MarkerSeverity.Error;
return {
- message: String(err.message || "Syntax error"),
- severity: monaco.MarkerSeverity.Error,
+ message: String(err.message || "Syntax error"),
+ severity: sev,
startLineNumber: line,
endLineNumber: line,
startColumn: 1,
endColumn: endCol,
};
});📝 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.
| const markers = (Array.isArray(errors) ? errors : []).map((err) => { | |
| const line = Number(err.lineNumber) > 0 ? Number(err.lineNumber) : 1; | |
| const endCol = model ? model.getLineMaxColumn(line) : 100; | |
| return { | |
| message: String(err.message || "Syntax error"), | |
| severity: monaco.MarkerSeverity.Error, | |
| startLineNumber: line, | |
| endLineNumber: line, | |
| startColumn: 1, | |
| endColumn: endCol, | |
| }; | |
| }); | |
| monaco.editor.setModelMarkers(model, "kidcode", markers); | |
| const markers = (Array.isArray(errors) ? errors : []).map((err) => { | |
| const rawLine = Number(err.lineNumber) || 1; | |
| const lineCount = model ? model.getLineCount() : 1; | |
| const line = Math.max(1, Math.min(rawLine, lineCount)); | |
| const endCol = model ? model.getLineMaxColumn(line) : 100; | |
| const sev = | |
| (err.severity === "warning" && monaco.MarkerSeverity.Warning) || | |
| (err.severity === "info" && monaco.MarkerSeverity.Info) || | |
| monaco.MarkerSeverity.Error; | |
| return { | |
| message: String(err.message || "Syntax error"), | |
| severity: sev, | |
| startLineNumber: line, | |
| endLineNumber: line, | |
| startColumn: 1, | |
| endColumn: endCol, | |
| }; | |
| }); | |
| monaco.editor.setModelMarkers(model, "kidcode", markers); |
🤖 Prompt for AI Agents
In kidcode-web/src/main/resources/static/app.js around lines 214 to 227, marker
line numbers and columns are not clamped against the model bounds which can
cause getLineMaxColumn(line) to throw when err.lineNumber exceeds the model's
lineCount; clamp the computed line to the range [1, model.getLineCount()] before
calling getLineMaxColumn, then use that clamped line for startLineNumber and
endLineNumber and compute endColumn from model.getLineMaxColumn(clampedLine);
also allow severity from the error (falling back to monaco.MarkerSeverity.Error)
and ensure startColumn is at least 1 and at most endColumn.
…r CodeRabbit review" This reverts commit 458485e.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
kidcode-web/src/main/resources/static/app.js (1)
182-203: Improve marker robustness and severity mapping.The validation markers have several issues:
- Line numbers aren't clamped: If
err.lineNumberexceeds the model's line count, markers may not render correctly or could cause unexpected behavior.- Hardcoded endColumn: Setting
endColumn: 100doesn't respect actual line length.- Ignored severity: The code always uses
Errorseverity, ignoringerr.severityif the backend provides it.Apply this diff to improve robustness:
async function validateCode() { const code = editor.getValue(); try { const response = await fetch("/api/validate", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ code: code }), }); const errors = await response.json(); - const markers = errors.map((err) => ({ - message: err.message, - severity: monaco.MarkerSeverity.Error, - startLineNumber: err.lineNumber, - endLineNumber: err.lineNumber, - startColumn: 1, - endColumn: 100, - })); + const model = editor.getModel(); + const lineCount = model ? model.getLineCount() : 1; + + const markers = errors.map((err) => { + const rawLine = Number(err.lineNumber) || 1; + const line = Math.max(1, Math.min(rawLine, lineCount)); + const endCol = model ? model.getLineMaxColumn(line) : 100; + + // Map severity string to Monaco severity enum + let severity = monaco.MarkerSeverity.Error; + if (err.severity === "warning") { + severity = monaco.MarkerSeverity.Warning; + } else if (err.severity === "info") { + severity = monaco.MarkerSeverity.Info; + } + + return { + message: err.message || "Syntax error", + severity: severity, + startLineNumber: line, + endLineNumber: line, + startColumn: 1, + endColumn: endCol, + }; + }); monaco.editor.setModelMarkers(editor.getModel(), "kidcode", markers); } catch (error) { console.error("Validation request failed:", error); } }
🧹 Nitpick comments (1)
kidcode-web/src/main/resources/static/style.css (1)
398-400: Remove unnecessary trailing blank lines.The extra blank lines after the closing brace serve no functional purpose and should be removed for consistency.
Apply this diff:
#help-docs th { - background-color: #f2f2f2; -} - - + background-color: #f2f2f2; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kidcode-web/src/main/resources/static/app.js(1 hunks)kidcode-web/src/main/resources/static/style.css(3 hunks)
🔇 Additional comments (5)
kidcode-web/src/main/resources/static/style.css (1)
37-125: LGTM! Well-structured panel header layout.The panel header CSS is well-organized with:
- Clear separation of concerns (left panel for title/dropdown, right panel for controls)
- Responsive flexbox layout with
nowrapto prevent unwrapping- Accessible
.sr-onlyutility following best practices- Smooth transitions and hover states for the dropdown
The implementation effectively supports the new example selector feature.
kidcode-web/src/main/resources/static/app.js (4)
129-158: LGTM! Example initialization properly isolated.The
initializeExamplesfunction successfully addresses the previous review concerns:
- Checks for selector element existence before proceeding
- Gracefully degrades when
examples.jsis missing by disabling the selector- Doesn't block core editor functionality with early returns
- Provides clear console warnings for debugging
This ensures the editor remains functional even if
examples.jsfails to load.
18-63: LGTM! Well-structured tokenizer definition.The Monaco language registration has been properly refactored:
- Keywords are organized in a clear array
- Tokenizer states (root and string) are well-defined
- String handling includes escape sequences and invalid cases
- Comment and number patterns are correctly specified
This provides robust syntax highlighting for the KidCode language.
70-127: LGTM! Editor initialization flow is solid.The Monaco editor setup demonstrates good practices:
- Clear initialization sequence using require.config
- Reasonable default code showcasing language features
- Proper integration of example loading via
initializeExamples()- Keyboard shortcut (Ctrl/Cmd+Enter) for running code enhances UX
- Debounced live validation (500ms) prevents excessive API calls
The initialization order ensures core features are always available.
246-297: LGTM! Event rendering and canvas management are robust.The rendering logic correctly:
- Handles different event types (Clear, Move, Say, Error)
- Maintains state for lines and Cody position
- Only draws lines when pen is down and position changes
- Properly separates event processing from canvas redraw
- Updates canvas efficiently by redrawing all lines then Cody
This design allows for smooth animation and proper state management.
| .panel-controls { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 14px; | ||
| } | ||
|
|
||
| /* Ensure controls don't wrap and keep their size */ | ||
| .panel-controls { | ||
| flex-shrink: 0; | ||
| white-space: nowrap; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Merge duplicate .panel-controls rules.
The .panel-controls selector appears twice (lines 99-103 and 106-109), which can lead to confusion and maintenance issues. Consolidate these into a single rule.
Apply this diff to merge the rules:
-/* 🖲️ Right side: Buttons stay aligned */
-.panel-controls {
- display: flex;
- align-items: center;
- gap: 14px;
-}
-
-/* Ensure controls don't wrap and keep their size */
-.panel-controls {
- flex-shrink: 0;
- white-space: nowrap;
-}
+/* 🖲️ Right side: Buttons stay aligned and don't wrap */
+.panel-controls {
+ display: flex;
+ align-items: center;
+ gap: 14px;
+ flex-shrink: 0;
+ white-space: nowrap;
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In kidcode-web/src/main/resources/static/style.css around lines 99 to 109, there
are two duplicate .panel-controls rules; merge them into a single rule
containing all properties (display: flex; align-items: center; gap: 14px;
flex-shrink: 0; white-space: nowrap;) and remove the redundant block so
.panel-controls is defined once with the combined declarations.
|
Resolved all CodeRabbit review issues. |
|
Great job @ADITHICJ ! We are ready to merge once you resolve the merge conflicts. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
kidcode-web/src/main/resources/static/app.js (1)
250-257: Critical: Clamp marker line numbers to prevent runtime errors.
getLineMaxColumn(line)will throw iferr.lineNumberexceeds the model's line count. Line numbers must be clamped to[1, model.getLineCount()]before deriving the end column.Apply this diff to add bounds checking:
const errors = await response.json(); - const markers = errors.map((err) => ({ - message: err.message, - severity: monaco.MarkerSeverity.Error, - startLineNumber: err.lineNumber, - endLineNumber: err.lineNumber, - startColumn: 1, - endColumn: 100, - })); + const model = editor.getModel(); + const markers = (Array.isArray(errors) ? errors : []).map((err) => { + const rawLine = Number(err.lineNumber) || 1; + const lineCount = model ? model.getLineCount() : 1; + const line = Math.max(1, Math.min(rawLine, lineCount)); + const endCol = model ? model.getLineMaxColumn(line) : 100; + const sev = + (err.severity === "warning" && monaco.MarkerSeverity.Warning) || + (err.severity === "info" && monaco.MarkerSeverity.Info) || + monaco.MarkerSeverity.Error; + return { + message: String(err.message || "Syntax error"), + severity: sev, + startLineNumber: line, + endLineNumber: line, + startColumn: 1, + endColumn: endCol, + }; + }); monaco.editor.setModelMarkers(editor.getModel(), "kidcode", markers);Based on past review comments.
🧹 Nitpick comments (1)
kidcode-web/src/main/resources/static/style.css (1)
394-397: Optional: Remove extra trailing newlines.Two trailing newlines after the
#help-docs thrule (lines 395-397) create unnecessary whitespace. Consider removing them for consistency.Apply this diff:
#help-docs th { - background-color: #f2f2f2; -} - - + background-color: #f2f2f2; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
kidcode-web/src/main/resources/static/app.js(1 hunks)kidcode-web/src/main/resources/static/index.html(1 hunks)kidcode-web/src/main/resources/static/style.css(3 hunks)
🔇 Additional comments (4)
kidcode-web/src/main/resources/static/style.css (1)
37-121: LGTM! Panel header layout is well-structured.The panel header CSS provides a clean, responsive layout with proper accessibility support through the
.sr-onlyutility class. The dropdown styling includes appropriate interactive states, and the previous duplicate.panel-controlsrules have been successfully consolidated.kidcode-web/src/main/resources/static/index.html (2)
29-32: LGTM! Accessible label successfully added.The
aria-label="Choose Example"attribute properly addresses the previous accessibility concern, ensuring screen readers can announce meaningful context for the dropdown control.
249-250: LGTM! Correct script loading order.Loading
examples.jsbeforeapp.jsensures thewindow.examplesobject is available wheninitializeExamples()runs, preventing initialization errors.kidcode-web/src/main/resources/static/app.js (1)
149-178: LGTM! Proper defensive checks for example loading.The
initializeExamples()function correctly implements graceful degradation with proper checks for:
- Missing selector element
- Missing
window.examplesobject- Appropriate fallback UI (disabled dropdown with message)
This addresses previous review concerns about defensive programming and ensures the editor remains functional even if
examples.jsfails to load.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
kidcode-web/src/main/resources/static/app.js (3)
146-146: Add clarifying comment for immediate validation call.The immediate
validateCode()call bypasses the debounce logic. If this is intentional (to validate on initial load), add a brief comment:validationTimeout = setTimeout(validateCode, 500); }); + // Validate on initial load validateCode();
15-15: Critical: Duplicate KIDCODE_STORAGE_KEY definition still present.The duplicate
KIDCODE_STORAGE_KEYconstant flagged in previous reviews remains unfixed:
- Line 15:
"kidcode.savedCode"(unused, shadowed)- Line 78:
"kidcode_saved_code"(actually used)The inner definition shadows the outer one, and both localStorage operations use the inner key. Remove the duplicate at line 78:
// --- Load code from localStorage or use default --- - const KIDCODE_STORAGE_KEY = "kidcode_saved_code"; - const savedCode = localStorage.getItem(KIDCODE_STORAGE_KEY);Also applies to: 78-78
233-240: Critical: Marker line numbers must be clamped to model bounds.The validation marker creation doesn't clamp line numbers to the model's valid range. If the API returns a
lineNumberexceeding the model's line count, this will cause a runtime error.Additionally,
endColumnis hardcoded to 100, which may not match the actual line length.Apply this diff to clamp line numbers and compute proper column bounds:
- const markers = errors.map((err) => ({ - message: err.message, - severity: monaco.MarkerSeverity.Error, - startLineNumber: err.lineNumber, - endLineNumber: err.lineNumber, - startColumn: 1, - endColumn: 100, - })); + const model = editor.getModel(); + const lineCount = model ? model.getLineCount() : 1; + const markers = errors.map((err) => { + const rawLine = Number(err.lineNumber) || 1; + const line = Math.max(1, Math.min(rawLine, lineCount)); + const endCol = model ? model.getLineMaxColumn(line) : 100; + return { + message: err.message || "Syntax error", + severity: monaco.MarkerSeverity.Error, + startLineNumber: line, + endLineNumber: line, + startColumn: 1, + endColumn: endCol, + }; + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
kidcode-web/src/main/resources/static/app.js(1 hunks)
🔇 Additional comments (6)
kidcode-web/src/main/resources/static/app.js (6)
23-66: LGTM! Well-structured tokenizer configuration.The expanded tokenizer properly handles keywords, numbers, comments, and strings with appropriate state transitions. The string state correctly manages escaped characters and invalid strings.
149-178: LGTM! Excellent defensive programming.The
initializeExamplesfunction properly handles missing DOM elements and unavailable examples data with graceful degradation. The disabled state with a helpful message provides good user feedback.
182-205: LGTM! Proper execution flow with error handling.The run button handler correctly:
- Clears the canvas before execution
- Sends code to the API
- Handles HTTP errors
- Renders returned events
208-221: LGTM! Safe download implementation.The download button handler includes proper error handling with try-catch and provides user feedback through logging.
248-339: LGTM! Well-structured rendering pipeline.The rendering functions properly manage canvas state, handle different event types, and maintain Cody's position and drawing history. The separation of concerns between event processing and canvas redrawing is clean.
341-353: LGTM! Standard modal interaction.Help modal event listeners correctly handle open/close actions including the backdrop click-to-close pattern.
|
@Sansi-28 All CodeRabbit review issues have been resolved. The branch is ready for merge. |
|
Merged! Keep Contributing @ADITHICJ |
2 participants
Overview
This update enhances the KidCode Web Editor by adding a new Example Dropdown Menu that helps users explore pre-written KidCode scripts easily.
Users can now select a script example from the dropdown, and it will automatically load into the Monaco editor for instant execution.
This feature improves learning and experimentation, especially for new coders exploring the KidCode language.
What's New
Technical Details
Updated app.js to:
How It Works
Outcome
This feature makes KidCode more interactive, educational, and beginner-friendly, allowing users to explore coding concepts visually with minimal typing.
Tested On
Summary by CodeRabbit
New Features
Enhancements
Style
Bug Fixes