Added help modal #1
Conversation
📝 WalkthroughWalkthroughThis change adds a standalone Help Modal system and integrates it into the app. New files include help-modal.js (global HelpModal), help-content.html (static help content), and extended modal styling in style.css. index.html now uses a button for Help and includes help-modal.js before app.js. app.js asynchronously fetches help-content.html and initializes HelpModal with a trigger selector and theme, with warning logs on failure. Documentation is added in HELP-MODAL-README.md. solution.json updates the schema: renamed root, removed version, redefined tables (users, peep), adjusted columns and constraints, updated positions/colors, and cleared foreignKeys. Poem
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
style.css (1)
275-281: Prevent scroll chaining/overscroll in the modal body.This avoids background page bounce on mobile/trackpads.
.help-modal-body { padding: 24px; overflow-y: auto; flex: 1; line-height: 1.6; + overscroll-behavior: contain; }help-modal.js (1)
14-20: Remove or implement unused options (theme, customStyles).Options are documented but unused; either wire them up or trim from API/docs to avoid drift.
Would you like me to wire theme via a data attribute (e.g., data-theme on html) and apply customStyles inline on the modal surface?
app.js (1)
449-461: LGTM. Initialize HelpModal with fetched content and graceful fallback.Looks good. Optional: keep a reference to the instance if you plan to control it programmatically (e.g., open from other UI).
- HelpModal.init({ + window.helpModal = HelpModal.init({ triggerSelector: '#btn-help', content: helpContent, theme: 'auto' });index.html (1)
14-14: Add aria-haspopup to the Help trigger.Small a11y hint for assistive tech; pairs with role="dialog" in the modal.
- <button id="btn-help" class="as-button ghost">HELP</button> + <button id="btn-help" class="as-button ghost" aria-haspopup="dialog">HELP</button>HELP-MODAL-README.md (1)
36-43: Docs/API drift — implement or remove 'theme' and 'customStyles'.
help-modal.js sets theme and customStyles in the constructor but never uses them; either implement option handling (apply theme switching and merge customStyles into the modal) or remove them from README/examples.
Location: help-modal.js (constructor / default options).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
HELP-MODAL-README.md(1 hunks)app.js(1 hunks)help-content.html(1 hunks)help-modal.js(1 hunks)index.html(2 hunks)solution.json(1 hunks)style.css(1 hunks)
🔇 Additional comments (3)
index.html (1)
110-111: Script order is correct.help-modal.js loads before app.js, so initialization can run. Nice.
help-content.html (1)
1-105: LGTM. Structured, linkable sections with semantic elements.Content is static and safe to inject. No scripts present. Works with internal anchor handling.
solution.json (1)
2-73: Confirm solution.json changes are intentional.This PR focuses on a Help Modal, but the schema/data changed (tables renamed, FKs cleared, positions updated). If unintentional, please revert to avoid mixing concerns. Also note: unique constraint duplicates the primary key on both tables.
| 1. Copy `help-modal.js` to your project | ||
| 2. Copy the modal styles from `style.css` (lines 202-410) | ||
| 3. Adapt the CSS custom properties to match your theme | ||
| 4. Create your help content | ||
| 5. Initialize with `HelpModal.init()` | ||
|
|
There was a problem hiding this comment.
Avoid brittle line-number references to CSS.
Point to the named block instead.
-2. Copy the modal styles from `style.css` (lines 202-410)
+2. Copy the CSS block labeled “Help Modal Styles” from `style.css`📝 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.
| 1. Copy `help-modal.js` to your project | |
| 2. Copy the modal styles from `style.css` (lines 202-410) | |
| 3. Adapt the CSS custom properties to match your theme | |
| 4. Create your help content | |
| 5. Initialize with `HelpModal.init()` | |
| 1. Copy `help-modal.js` to your project | |
| 2. Copy the CSS block labeled “Help Modal Styles” from `style.css` | |
| 3. Adapt the CSS custom properties to match your theme | |
| 4. Create your help content | |
| 5. Initialize with `HelpModal.init()` |
🤖 Prompt for AI Agents
In HELP-MODAL-README.md around lines 70 to 75, the README currently refers to
CSS by brittle line numbers (lines 202-410); update the instruction to point to
the named CSS block or selectors instead — e.g., reference the .help-modal
styles block and the CSS custom properties (or the comment header "Help Modal
styles" in style.css) so integrators copy the .help-modal selector and the
related :root/custom-property declarations rather than relying on line numbers.
| this.modal.innerHTML = ` | ||
| <div class="help-modal-backdrop"></div> | ||
| <div class="help-modal-content"> | ||
| <div class="help-modal-header"> | ||
| <h2>Help / User Guide</h2> | ||
| <button class="help-modal-close" type="button" aria-label="Close help">×</button> | ||
| </div> | ||
| <div class="help-modal-body"> | ||
| ${this.options.content} | ||
| </div> | ||
| </div> | ||
| `; |
There was a problem hiding this comment.
Add dialog semantics (role, aria) and label the heading.
Improves screen reader support.
- <div class="help-modal-backdrop"></div>
- <div class="help-modal-content">
+ <div class="help-modal-backdrop"></div>
+ <div class="help-modal-content" role="dialog" aria-modal="true" aria-labelledby="help-modal-title" tabindex="-1">
<div class="help-modal-header">
- <h2>Help / User Guide</h2>
+ <h2 id="help-modal-title">Help / User Guide</h2>
<button class="help-modal-close" type="button" aria-label="Close help">×</button>
</div>
<div class="help-modal-body">
${this.options.content}
</div>
</div>📝 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.
| this.modal.innerHTML = ` | |
| <div class="help-modal-backdrop"></div> | |
| <div class="help-modal-content"> | |
| <div class="help-modal-header"> | |
| <h2>Help / User Guide</h2> | |
| <button class="help-modal-close" type="button" aria-label="Close help">×</button> | |
| </div> | |
| <div class="help-modal-body"> | |
| ${this.options.content} | |
| </div> | |
| </div> | |
| `; | |
| this.modal.innerHTML = ` | |
| <div class="help-modal-backdrop"></div> | |
| <div class="help-modal-content" role="dialog" aria-modal="true" aria-labelledby="help-modal-title" tabindex="-1"> | |
| <div class="help-modal-header"> | |
| <h2 id="help-modal-title">Help / User Guide</h2> | |
| <button class="help-modal-close" type="button" aria-label="Close help">×</button> | |
| </div> | |
| <div class="help-modal-body"> | |
| ${this.options.content} | |
| </div> | |
| </div> | |
| `; |
🤖 Prompt for AI Agents
In help-modal.js around lines 38 to 49, the modal markup lacks dialog semantics
and a labeled heading for screen readers; add role="dialog" and
aria-modal="true" to the container element, add an id to the <h2> (e.g.
id="help-modal-title") and add aria-labelledby="help-modal-title" to the dialog
container so the heading is announced, and keep the existing close button
aria-label as-is; ensure the aria-labelledby value matches the heading id.
| // ESC key | ||
| document.addEventListener('keydown', (e) => { | ||
| if (e.key === 'Escape' && this.isOpen) { | ||
| this.close(); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
Trap focus, scope/remove keydown listener, and safely restore body overflow.
Prevents keyboard escape/focus leaks and restores original page overflow. Also inert the background while open.
// ESC key
- document.addEventListener('keydown', (e) => {
- if (e.key === 'Escape' && this.isOpen) {
- this.close();
- }
- });
+ // (Moved to open/close lifecycle; see below) open() {
if (this.isOpen) return;
this.isOpen = true;
this.modal.style.display = 'block';
- document.body.style.overflow = 'hidden'; // Prevent background scrolling
+ // Prevent background scrolling (preserve previous value)
+ this._prevOverflow = document.body.style.overflow;
+ document.body.style.overflow = 'hidden';
+
+ // Inert all siblings while dialog is open
+ this._inertSiblings = Array.from(document.body.children).filter(el => el !== this.modal);
+ this._inertSiblings.forEach(el => {
+ try { el.inert = true; } catch (_) {}
+ el.setAttribute('aria-hidden', 'true');
+ });
+
+ // Keydown (ESC + focus trap) active only while open
+ this._onKeydown = (e) => {
+ if (e.key === 'Escape') { this.close(); return; }
+ if (e.key === 'Tab') this._trapFocus(e);
+ };
+ document.addEventListener('keydown', this._onKeydown, true);
// Focus management
const closeBtn = this.modal.querySelector('.help-modal-close');
- closeBtn.focus();
+ (closeBtn || this.modal).focus();
// Trigger custom event
this.trigger.dispatchEvent(new CustomEvent('helpModal:open', { detail: this }));
} close() {
if (!this.isOpen) return;
this.isOpen = false;
this.modal.style.display = 'none';
- document.body.style.overflow = ''; // Restore scrolling
+ // Restore scrolling
+ document.body.style.overflow = this._prevOverflow || '';
+ this._prevOverflow = '';
+
+ // Un-inert siblings
+ if (this._inertSiblings) {
+ this._inertSiblings.forEach(el => {
+ try { el.inert = false; } catch (_) {}
+ el.removeAttribute('aria-hidden');
+ });
+ this._inertSiblings = null;
+ }
+
+ // Remove keydown handler
+ if (this._onKeydown) {
+ document.removeEventListener('keydown', this._onKeydown, true);
+ this._onKeydown = null;
+ }
// Return focus to trigger
- this.trigger.focus();
+ this.trigger && this.trigger.focus();
// Trigger custom event
this.trigger.dispatchEvent(new CustomEvent('helpModal:close', { detail: this }));
} destroy() {
if (this.modal && this.modal.parentNode) {
this.modal.parentNode.removeChild(this.modal);
}
- document.body.style.overflow = '';
+ // Cleanup listeners and page state
+ if (this._onKeydown) {
+ document.removeEventListener('keydown', this._onKeydown, true);
+ this._onKeydown = null;
+ }
+ document.body.style.overflow = this._prevOverflow || '';
+ this._prevOverflow = '';
}Add these utilities to the class (e.g., after constructor or near other methods):
class HelpModal {
constructor(options = {}) {
@@
this.isOpen = false;
this.modal = null;
this.trigger = null;
+ this._onKeydown = null;
+ this._prevOverflow = '';
+ this._inertSiblings = null;
+ this._focusables = 'a[href], button:not([disabled]), textarea, input, select, [tabindex]:not([tabindex="-1"])';
@@
}
@@
+ _trapFocus(e) {
+ const nodes = this.modal.querySelectorAll(this._focusables);
+ if (!nodes.length) return;
+ const first = nodes[0];
+ const last = nodes[nodes.length - 1];
+ if (e.shiftKey) {
+ if (document.activeElement === first) { e.preventDefault(); last.focus(); }
+ } else {
+ if (document.activeElement === last) { e.preventDefault(); first.focus(); }
+ }
+ }Also applies to: 102-115, 117-129, 136-141
🤖 Prompt for AI Agents
In help-modal.js around lines 82 to 88 (and also apply changes to ranges
102-115, 117-129, 136-141), the modal currently adds a global keydown listener
for Escape without scoping/removal, doesn't trap focus, doesn't make the
background inert, and mutates body overflow unsafely; update the class to: add
methods to add/remove a scoped keydown listener that is registered on open and
removed on close, implement focus trapping (store previously focused element,
move focus into the modal on open, constrain Tab/Shift+Tab within focusable
elements, and restore focus on close), make the background inert/aria-hidden
while open, and change body overflow handling to save the original overflow
value and restore it on close (avoid direct blind overrides); add these utility
methods near the constructor and call them from the modal open/close paths so
listeners and state are cleaned up reliably.
| this.modal.addEventListener('click', (e) => { | ||
| if (e.target.matches('a[href^="#"]')) { | ||
| e.preventDefault(); | ||
| const targetId = e.target.getAttribute('href').substring(1); | ||
| const targetElement = this.modal.querySelector(`#${targetId}`); | ||
| if (targetElement) { | ||
| targetElement.scrollIntoView({ behavior: 'smooth' }); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Use delegated link handling via closest() and respect reduced motion.
Fixes clicks on nested elements inside anchors and honors prefers-reduced-motion.
- this.modal.addEventListener('click', (e) => {
- if (e.target.matches('a[href^="#"]')) {
- e.preventDefault();
- const targetId = e.target.getAttribute('href').substring(1);
- const targetElement = this.modal.querySelector(`#${targetId}`);
- if (targetElement) {
- targetElement.scrollIntoView({ behavior: 'smooth' });
- }
- }
- });
+ this.modal.addEventListener('click', (e) => {
+ const link = e.target.closest('a[href^="#"]');
+ if (!link) return;
+ e.preventDefault();
+ const id = link.getAttribute('href').slice(1);
+ const target = this.modal.querySelector(`#${CSS.escape(id)}`);
+ if (!target) return;
+ const smooth = window.matchMedia('(prefers-reduced-motion: reduce)').matches ? 'auto' : 'smooth';
+ target.scrollIntoView({ behavior: smooth });
+ });📝 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.
| this.modal.addEventListener('click', (e) => { | |
| if (e.target.matches('a[href^="#"]')) { | |
| e.preventDefault(); | |
| const targetId = e.target.getAttribute('href').substring(1); | |
| const targetElement = this.modal.querySelector(`#${targetId}`); | |
| if (targetElement) { | |
| targetElement.scrollIntoView({ behavior: 'smooth' }); | |
| } | |
| } | |
| }); | |
| this.modal.addEventListener('click', (e) => { | |
| const link = e.target.closest('a[href^="#"]'); | |
| if (!link) return; | |
| e.preventDefault(); | |
| const id = link.getAttribute('href').slice(1); | |
| const target = this.modal.querySelector(`#${CSS.escape(id)}`); | |
| if (!target) return; | |
| const smooth = window.matchMedia('(prefers-reduced-motion: reduce)').matches ? 'auto' : 'smooth'; | |
| target.scrollIntoView({ behavior: smooth }); | |
| }); |
🤖 Prompt for AI Agents
In help-modal.js around lines 90 to 99, the click handler currently only detects
direct anchor clicks and always uses smooth scrolling; update it to use
event.target.closest('a[href^="#"]') so clicks on nested elements inside an
anchor are recognized, only call e.preventDefault() when such an anchor is
found, extract and sanitize the target id from the anchor's href, query the
modal for that id and call scrollIntoView; respect user reduced-motion
preferences by detecting window.matchMedia('(prefers-reduced-motion:
reduce)').matches and pass behavior: 'auto' when reduced motion is requested
otherwise 'smooth'.
| .help-modal-close { | ||
| background: none; | ||
| border: none; | ||
| font-size: 24px; | ||
| color: var(--muted); | ||
| cursor: pointer; | ||
| padding: 4px 8px; | ||
| border-radius: 4px; | ||
| line-height: 1; | ||
| transition: all 0.2s ease; | ||
| } | ||
|
|
||
| .help-modal-close:hover { | ||
| background: var(--control-bg); | ||
| color: var(--fg); | ||
| } |
There was a problem hiding this comment.
Add visible focus style for the close button (keyboard accessibility).
Currently only hover is styled; add a clear focus-visible outline.
.help-modal-close {
background: none;
border: none;
font-size: 24px;
color: var(--muted);
cursor: pointer;
padding: 4px 8px;
border-radius: 4px;
line-height: 1;
transition: all 0.2s ease;
}
+.help-modal-close:focus-visible {
+ outline: 2px solid var(--accent);
+ outline-offset: 2px;
+}
.help-modal-close:hover {
background: var(--control-bg);
color: var(--fg);
}🤖 Prompt for AI Agents
In style.css around lines 258 to 273, the close button only has hover styles and
lacks a visible focus style for keyboard users; add a :focus-visible rule (e.g.
.help-modal-close:focus-visible) that applies a clear outline or box-shadow (use
a high-contrast color variable such as var(--accent) or var(--focus)), set
outline-offset: 2px and preserve border-radius and padding so the ring sits
outside the element, and ensure the rule does not affect mouse hover behavior so
only keyboard focus shows the visible indicator.
No description provided.