-
Notifications
You must be signed in to change notification settings - Fork 0
Add BACKLOG.md with prioritized improvement tasks for future agents #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,244 @@ | ||||||||||||||||||||||||||
| # BACKLOG.md — Improvement Tasks for Future Agents | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| This document captures prioritized, actionable improvement tasks identified | ||||||||||||||||||||||||||
| through a comprehensive code analysis. Each task includes the problem, affected | ||||||||||||||||||||||||||
| files/lines, and a concrete implementation plan. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Ground rules for agents working on these tasks:** | ||||||||||||||||||||||||||
| - Run `node .claude/hooks/run-tests.js` before and after every change | ||||||||||||||||||||||||||
| - Add tests in `tests.html` for any new logic | ||||||||||||||||||||||||||
| - Keep the zero-dependency, no-build-step constraint | ||||||||||||||||||||||||||
| - Do not add npm packages or bundlers | ||||||||||||||||||||||||||
| - All JS must work without transpilation (no JSX, no ES modules) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ## Task 1 — Add React Error Boundary and Crash Recovery | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Priority:** HIGH | **Effort:** Low | **Category:** Reliability | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Problem:** No error boundary exists. If any React component throws, the entire | ||||||||||||||||||||||||||
| app goes blank with no recovery path. Users must hard-refresh and may not | ||||||||||||||||||||||||||
| understand what happened. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Files:** | ||||||||||||||||||||||||||
| - `index.html` — wrap App component (around line 1295) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Implementation:** | ||||||||||||||||||||||||||
| 1. Add an `ErrorBoundary` class component before the App definition: | ||||||||||||||||||||||||||
| ```js | ||||||||||||||||||||||||||
| class ErrorBoundary extends React.Component { | ||||||||||||||||||||||||||
| constructor(props) { super(props); this.state = { hasError: false, error: null }; } | ||||||||||||||||||||||||||
| static getDerivedStateFromError(error) { return { hasError: true, error: error }; } | ||||||||||||||||||||||||||
| componentDidCatch(error, info) { console.error('App crash:', error, info); } | ||||||||||||||||||||||||||
| render() { | ||||||||||||||||||||||||||
| if (this.state.hasError) { | ||||||||||||||||||||||||||
| return React.createElement('div', { style: { padding: 40, textAlign: 'center' } }, | ||||||||||||||||||||||||||
| React.createElement('h2', null, 'Something went wrong'), | ||||||||||||||||||||||||||
| React.createElement('p', null, this.state.error && this.state.error.message), | ||||||||||||||||||||||||||
| React.createElement('button', { onClick: function() { location.reload(); } }, 'Reload'), | ||||||||||||||||||||||||||
| React.createElement('button', { onClick: function() { localStorage.clear(); location.reload(); } }, 'Reset all data') | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| React.createElement('button', { onClick: function() { localStorage.clear(); location.reload(); } }, 'Reset all data') | |
| React.createElement('button', { onClick: function() { | |
| try { | |
| ['n5_2025', 'n5_srs', 'n5_day', 'n5_completed', 'n5_furigana'].forEach(function(key) { | |
| localStorage.removeItem(key); | |
| }); | |
| } catch (e) { | |
| console.error('Failed to reset app data', e); | |
| } | |
| location.reload(); | |
| } }, 'Reset all data') |
Copilot
AI
Mar 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Task 1 step 2 uses JSX-style <App /> / <ErrorBoundary> wording, but the repository explicitly avoids JSX/ES modules. Suggest updating the instruction to show the equivalent React.createElement(ErrorBoundary, …, React.createElement(App)) wrapper so future agents don’t accidentally introduce JSX.
| 2. Wrap `<App />` in `<ErrorBoundary>` in the `ReactDOM.createRoot` call | |
| 2. Wrap `App` in `ErrorBoundary` in the `ReactDOM.createRoot` call, for example: | |
| ```js | |
| ReactDOM.createRoot(root).render( | |
| React.createElement( | |
| ErrorBoundary, | |
| null, | |
| React.createElement(App) | |
| ) | |
| ); |
Copilot
AI
Mar 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Task 3, adding role="button" to actual <button> elements is redundant and can be counterproductive; focus on accessible naming (aria-label), state (aria-pressed for toggles), and keyboard handlers where the element is not a native button. Also tabIndex={0} is JSX syntax and conflicts with the “no JSX” rule—use the non-JSX prop form (e.g., tabIndex: 0) in createElement calls.
| 2. **Quiz buttons:** Add `role="button"` and `aria-label` with the answer text | |
| 3. **Feedback messages:** Add `aria-live="polite"` to exercise result text | |
| 4. **Navigation:** Add `role="navigation"` to header, `role="main"` to content area | |
| 5. **Review cards:** Make card flippable via Enter/Space key (add `tabIndex={0}` | |
| and `onKeyDown` handler) | |
| 2. **Quiz buttons:** Ensure each quiz answer control has an accessible name (e.g., | |
| an `aria-label` with the answer text); if it is not a native `<button>`, add | |
| `role="button"` and keyboard handlers for Enter/Space. | |
| 3. **Feedback messages:** Add `aria-live="polite"` to exercise result text | |
| 4. **Navigation:** Add `role="navigation"` to header, `role="main"` to content area | |
| 5. **Review cards:** Make card flippable via Enter/Space key (set `tabIndex: 0` | |
| and an `onKeyDown` handler via the `createElement` props) |
Copilot
AI
Mar 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSP snippet in Task 4 includes script-src 'unsafe-inline', which would still allow inline event-handler execution (one of the main SVG XSS risks) and therefore doesn’t meaningfully mitigate injected-SVG XSS. If CSP is intended as a defense-in-depth measure here, consider either (a) moving inline JS/CSS into external files so 'unsafe-inline' can be removed (or replaced with hashes/nonces), or (b) explicitly call out that this CSP is mainly about restricting remote origins and isn’t sufficient by itself.
| <meta http-equiv="Content-Security-Policy" content="default-src 'self' https://cdnjs.cloudflare.com; script-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline';"> | |
| <meta http-equiv="Content-Security-Policy" content="default-src 'self' https://cdnjs.cloudflare.com; script-src 'self' https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline';"> |
Copilot
AI
Mar 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Task 7 suggests extracting hardcoded colors into “CSS custom properties that reference the PHASE_COLORS constants,” but CSS can’t directly reference JS variables. To avoid confusion, consider rephrasing to either duplicate the values in CSS variables (and note they must be kept in sync) or set CSS variables at runtime from PHASE_COLORS (e.g., writing to document.documentElement.style).
| properties that reference the `PHASE_COLORS` constants | |
| properties, and either duplicate the corresponding `PHASE_COLORS` values (noting | |
| they must be kept in sync) or set those CSS variables at runtime from | |
| `PHASE_COLORS` (for example via `document.documentElement.style`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ground rules say to run
.claude/hooks/run-tests.jsand also to add tests intests.html, but the pre-commit hook only runs the Node runner. Adding a test only totests.htmlwon’t be enforced by the automated test gate unless it’s also added torun-tests.js(or the runner is updated to execute the browser suite). Consider clarifying this so contributors know where new tests must be added.