Feature/quiz feedback 704 #736
Conversation
- Add congratulatory messages for correct answers with explanations - Add hints for incorrect answers - Implement Try Again button to reset quiz - Add animations for correct/incorrect selections - Support both light and dark themes - Disable other options after selection Closes CircuitVerse#704
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| .quiz-feedback { | ||
| box-shadow: 0 2px 8px rgba(255, 255, 255, 0.1); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Files should end with a trailing newline
| } | ||
|
|
||
| .quiz-feedback { | ||
| box-shadow: 0 2px 8px rgba(255, 255, 255, 0.1); |
There was a problem hiding this comment.
Color literals like rgba(255, 255, 255, 0.1) should only be used in variable declarations; they should be referred to via variable everywhere else.
| border-color: #dc3545 !important; | ||
| } | ||
|
|
||
| .quiz-feedback { |
There was a problem hiding this comment.
Line should be indented 2 spaces, but was indented 4 spaces
| background-color: #3a1e1e !important; | ||
| border-color: #dc3545 !important; | ||
| } | ||
|
|
|
|
||
| .quiz-answer.quiz-show-answer.quiz-false { | ||
| background-color: #3a1e1e !important; | ||
| border-color: #dc3545 !important; |
There was a problem hiding this comment.
!important should not be used
Color literals like #dc3545 should only be used in variable declarations; they should be referred to via variable everywhere else.
| border-color: #dc3545; | ||
| color: #f5b7bd; | ||
| } | ||
|
|
| .feedback-incorrect { | ||
| background-color: #3a1e1e; | ||
| border-color: #dc3545; | ||
| color: #f5b7bd; |
There was a problem hiding this comment.
Color literals like #f5b7bd should only be used in variable declarations; they should be referred to via variable everywhere else.
|
|
||
| .feedback-incorrect { | ||
| background-color: #3a1e1e; | ||
| border-color: #dc3545; |
There was a problem hiding this comment.
Color literals like #dc3545 should only be used in variable declarations; they should be referred to via variable everywhere else.
| } | ||
|
|
||
| .feedback-incorrect { | ||
| background-color: #3a1e1e; |
There was a problem hiding this comment.
Color literals like #3a1e1e should only be used in variable declarations; they should be referred to via variable everywhere else.
| color: #7fdb9f; | ||
| } | ||
|
|
||
| .feedback-incorrect { |
There was a problem hiding this comment.
Line should be indented 2 spaces, but was indented 4 spaces
WalkthroughCopyright years updated from 2024 to 2025 in documentation files. New SCSS file added for quiz feedback styling with animations and dark mode support. Quiz JavaScript refactored to collect answers with explanations from HTML lists, implement delegated event handling, display per-answer feedback containers, and provide reset functionality. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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: 1
🧹 Nitpick comments (4)
README.md (1)
197-198: Copyright year bump looks goodThe update to 2025 is consistent with the rest of the PR; no issues from a code/doc standpoint. As a follow‑up (not blocking), you may want to later extend the “Pop Quizzes” section above to mention the new per‑answer explanations/hints format so authors know how to provide them.
_sass/quiz-feedback.scss (1)
5-33: Answered/disabled answer states and correctness styling align with JS behaviorThe
.answeredand.quiz-disabledrules correctly reinforce the JS logic (no re‑selection, dimmed alternatives, clear correct/incorrect highlighting). There is a small overlap between.quiz-question.answered .quiz-answerand.quiz-answer.quiz-disabledopacity handling, but it’s benign; you can keep it as‑is or consolidate later for tidiness.assets/js/quiz.js (2)
83-149: ShowQuizAnswer flow matches the new UX (lock answers, show feedback, allow retry)The event-based
ShowQuizAnswercorrectly locks the question after one selection, visually distinguishes the chosen answer, disables the rest, and displays either an explanation or hint plus a “Try Again” button, then scrolls to the feedback. The use ofscrollIntoViewwith smooth behavior is a reasonable enhancement; if it’s unsupported the feedback still appears, so there’s no functional blocker here.
151-172: ResetQuiz and escapeHtml utilities are appropriate and align with the new flow
ResetQuizcleanly restores the question to its initial state (removing answered/disabled/show‑answer classes and clearing feedback), which pairs well with the “Try Again” button, andescapeHtmlis correctly implemented for the characters relevant to your feedback/answer text. Once you escape the answer text when rendering options, this helper will fully centralize your safety guarantees.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(1 hunks)_sass/quiz-feedback.scss(1 hunks)about.md(1 hunks)assets/css/just-the-docs-circuitverse.scss(1 hunks)assets/css/just-the-docs-circuitversedark.scss(1 hunks)assets/js/quiz.js(3 hunks)
🔇 Additional comments (6)
about.md (1)
10-10: About page copyright year update is correctThe 2025 copyright line aligns with README and the project state; no additional changes needed here.
assets/css/just-the-docs-circuitversedark.scss (1)
7-9: Dark theme import order for quiz feedback looks correctAdding
@import "quiz-feedback";at the end ensures the new quiz feedback styles (including dark‑mode overrides) are available and can override earlier rules where needed. No issues here.assets/css/just-the-docs-circuitverse.scss (1)
3-5: Quiz feedback styles correctly wired into light themeImporting
quiz-feedbackat the end of the light theme stylesheet is appropriate and keeps feedback visuals consistent with the dark theme integration._sass/quiz-feedback.scss (2)
34-129: Animations, feedback box, and reset button styles are cohesiveThe pulse/shake and fade‑in animations, feedback box styling, and reset button states form a clear, consistent feedback UI. Focus styles are left to browser defaults, which keeps keyboard accessibility intact; no blocking issues here.
131-158: Dark‑mode overrides correctly target feedback and answer statesThe
body.dark-modeoverrides for feedback boxes and shown correct/incorrect answers map cleanly to the classes used in the JS and should preserve contrast on dark backgrounds. This integration looks sound.assets/js/quiz.js (1)
65-80: No legacyShowQuizAnswercalls found—event binding migration is completeVerification confirms the transition was done correctly. The function is defined with signature
ShowQuizAnswer(event)at line 83, extracting the element viaevent.currentTarget. The only reference in the entire codebase is the proper event binding at line 79 via$('.quiz-answer').on('click', ShowQuizAnswer). No direct calls with element arguments remain.
| // Get answers with explanations/hints | ||
| var answers = []; | ||
| $(this).find('ul').each(function() { | ||
|
|
||
| // Correct answers from <ol> with explanations | ||
| $(this).find('ol').each(function() { | ||
| $(this).children('li').each(function() { | ||
| answers.push([false, $(this).contents().get(0).nodeValue.trim()]); | ||
| var text = $(this).contents().get(0).nodeValue.trim(); | ||
| // Check for explanation in <em> tag, otherwise use default | ||
| var explanation = $(this).find('em').text() || | ||
| "Correct! Well done."; | ||
| answers.push([true, text, explanation]); | ||
| }); | ||
| }); | ||
| $(this).find('ol').each(function() { | ||
|
|
||
| // Incorrect answers from <ul> with hints | ||
| $(this).find('ul').each(function() { | ||
| $(this).children('li').each(function() { | ||
| answers.push([true, $(this).contents().get(0).nodeValue.trim()]); | ||
| var text = $(this).contents().get(0).nodeValue.trim(); | ||
| // Check for hint in <em> tag, otherwise use default | ||
| var hint = $(this).find('em').text() || | ||
| "That's not correct. Try again!"; | ||
| answers.push([false, text, hint]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Harden answer text extraction and escaping to avoid edge‑case crashes and XSS
The new logic for pulling answer text and explanations/hints largely meets the UX goals, but it’s brittle and slightly unsafe:
$(this).contents().get(0).nodeValue.trim();will throw if the first child is not a text node (e.g., if an author wraps the answer in<strong>or there is no text node at index 0), becausenodeValuemay benullorget(0)may beundefined.- The answer text (
answer[1]) is later interpolated directly into an HTML string without escaping. Since it originates from a text node, this is usually fine, but encoded sequences like<img ...>would become real HTML when reinserted, which can re‑open injection surface.
To make this more robust you can:
- Derive the answer text from the
<li>while stripping any<em>explanation node, then use.text().trim()on the remainder. - Escape the answer text when building
answerDiv(similar to how you already escape the feedback).
For example:
- var text = $(this).contents().get(0).nodeValue.trim();
+ // Extract visible answer text, excluding any <em> explanation node
+ var $li = $(this).clone();
+ $li.children('em').remove();
+ var text = $li.text().trim();
@@
- var answerDiv = $('<div class="' + classes + '" data-feedback="' +
- escapeHtml(answer[2]) + '">' +
- answer[1] +
- '</div>');
+ var answerDiv = $('<div class="' + classes + '" data-feedback="' +
+ escapeHtml(answer[2]) + '">' +
+ escapeHtml(answer[1]) +
+ '</div>');This keeps the authoring experience the same but guards against malformed markup and unintended HTML interpretation.
Also applies to: 57-63
🤖 Prompt for AI Agents
In assets/js/quiz.js around lines 18 to 41 (and also apply same change at
57-63), extracting answer text using .contents().get(0).nodeValue is brittle and
unsafe and the extracted text is later interpolated into HTML unescaped;
instead, derive the answer text from the <li> itself by temporarily cloning the
<li> and removing any inner <em> explanation/hint node, then call .text().trim()
on that clone to get robust plain-text; finally, ensure the text is HTML-escaped
when you build the answerDiv (use your existing escape helper or implement an
escape function) before interpolating into the DOM to prevent unintended HTML
interpretation or XSS.
1 participant
Feature/quiz-feedback-704
Enhance Pop Quiz Feedback: Correct/Incorrect Messages, Explanations & Try Again (#704)
Fixes #704
Changes done:
Screenshots / Demo
Before:
After:
https://github.com/user-attachments/assets/c94b39dc-228f-494d-b56c-4939331fafe7
Summary by CodeRabbit
New Features
Documentation