feat(dashboard): explained terms card, layout fixes, support link#294
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an interactive "Explained Terms" dashboard module, localStorage-backed utilities for hiding and ordering terms, UI/layout tweaks to the dashboard page, i18n string additions, and small component styling/regex updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dashboard as ExplainedTermsCard
participant Storage as localStorage
participant AI as AIWordHelper
participant i18n as Translate
User->>Dashboard: Mount / open dashboard
Dashboard->>Storage: getHiddenTerms(), getTermOrder()
Storage-->>Dashboard: hidden set, saved order
Dashboard->>i18n: load strings
i18n-->>Dashboard: localized text
User->>Dashboard: Click term
Dashboard->>AI: open detail modal with term
AI-->>User: show explanation
User->>Dashboard: Drag term
Dashboard->>Storage: saveTermOrder(updatedOrder)
User->>Dashboard: Hide term
Dashboard->>Storage: hideTermFromDashboard(term)
User->>Dashboard: Restore term
Dashboard->>Storage: unhideTermFromDashboard(term)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
🤖 Fix all issues with AI agents
In `@frontend/app/`[locale]/dashboard/page.tsx:
- Around line 113-115: ExplainedTermsCard currently never shows the localized
empty state; update the component (ExplainedTermsCard) to check terms.length (or
hasTerms) and when zero render the two translation keys
dashboard.explainedTerms.empty and dashboard.explainedTerms.emptyHint (using the
component's existing i18n/translation hook, e.g., t()) instead of only the
title/subtitle so users see "You haven't saved any explanations yet" and the
hint "Select any term in Q&A to get an AI explanation"; ensure the conditional
branch replaces or augments the current render path that shows title/subtitle
when hasTerms is false.
In `@frontend/components/dashboard/ExplainedTermsCard.tsx`:
- Around line 152-160: The icon-only buttons (the drag handle using
GripVertical, the remove button and the restore button) lack accessible names;
add aria-label attributes to each icon-only <button> (e.g., the drag handle that
calls handleDragStart(term) and checks draggedTerm) using the existing i18n
helper (use t(...) with contextual keys like t('ariaDragHandle', { term }),
t('ariaHide', { term }) and t('ariaRestore', { term })) so screen readers
receive a descriptive label that includes the term; ensure these aria-labels are
present on the buttons that only render icons and update any related tests/props
that expect these buttons to be labeled.
- Around line 143-177: The current list rendering uses key={term} in terms.map
which breaks reconciliation and DnD when duplicate strings occur; change the
component to use a stable unique key (e.g. `${term}-${index}` or an explicit id)
and modify the drag/drop handlers to operate on indices rather than indexOf:
update the map to pass the index into onDragStart and onDrop (referencing
terms.map, key={term}, handleDragStart, handleDrop) and update
handleDragStart/handleDrop to accept and use the sourceIndex/targetIndex for
reordering; alternatively deduplicate terms when loading from getCachedTerms()
so keys remain unique.
In `@frontend/messages/en.json`:
- Line 1033: Fix the typo in the JSON string value for the "content" key by
replacing "sctrictly" with "strictly" so the sentence reads "...for strictly
necessary DevLovers platform functionality."; update the value of the "content"
property accordingly.
In `@frontend/messages/pl.json`:
- Line 253: The translation for the key "builtWith" (footer.builtWith) is
incorrect: it currently reads "Zbudowane wokół" ("Built around") but should
match the English "Built by"; update the Polish value to a proper equivalent
such as "Zbudowane przez" so the translation semantics align with en.json.
🧹 Nitpick comments (8)
frontend/messages/uk.json (1)
937-946: Ukrainian pluralization missing fortermCount.
"{count} термінів збережено"only works for counts ≥ 5. Ukrainian has three plural forms: "термін" (1), "терміни" (2–4), "термінів" (5+). If you're usingnext-intl, ICU{count, plural, ...}syntax handles this. Note thatcartWithItems(line 25) has the same pre-existing issue, so this is consistent with current patterns — but worth addressing across the board.Example ICU plural form
-"termCount": "{count} термінів збережено" +"termCount": "{count, plural, one {# термін збережено} few {# терміни збережено} other {# термінів збережено}}"frontend/components/header/MainSwitcher.tsx (1)
78-78: Consider extracting the growing condition into a named variable.This ternary condition now chains four function calls. A named boolean would improve readability.
♻️ Suggested refactor
+ const isFullWidthPath = isQa || isHome || isQuizzesPath(pathname) || isDashboardPath(pathname); + return ( - <main className={isQa || isHome || isQuizzesPath(pathname) || isDashboardPath(pathname) ? 'mx-auto' : 'mx-auto min-h-[80vh] px-6'}> + <main className={isFullWidthPath ? 'mx-auto' : 'mx-auto min-h-[80vh] px-6'}> {children} </main> );frontend/lib/ai/hiddenTerms.ts (1)
17-19: AddArray.isArrayguard after parsing to prevent corrupted localStorage data from producing unexpected results.If the stored value is not an array (e.g., a string from external tampering),
new Set("hello")would yieldSet { "h", "e", "l", "o" }instead of failing gracefully.🛡️ Proposed fix
const terms: string[] = JSON.parse(raw); + if (!Array.isArray(terms)) { + return new Set(); + } return new Set(terms);frontend/app/[locale]/dashboard/page.tsx (1)
99-106: Hardcoded Telegram URL — consider extracting to a config or environment variable.The support link
https://t.me/devloversteamis hardcoded. If this channel changes, it would require a code change and redeploy. Consider moving it to an environment variable or a shared constants file.frontend/components/dashboard/ExplainedTermsCard.tsx (4)
74-88: Side effect inside state updater function.
saveTermOrder(newTerms)on line 86 is a side effect (localStorage write) inside thesetTermsupdater callback. React may call updater functions multiple times (e.g., in Strict Mode). Move the persistence call outside the updater.♻️ Proposed fix
const handleDrop = (targetTerm: string) => { if (!draggedTerm || draggedTerm === targetTerm) { setDraggedTerm(null); return; } - setTerms(prevTerms => { + setTerms(prevTerms => { const newTerms = [...prevTerms]; const draggedIndex = newTerms.indexOf(draggedTerm); const targetIndex = newTerms.indexOf(targetTerm); if (draggedIndex === -1 || targetIndex === -1) { return prevTerms; } newTerms.splice(draggedIndex, 1); newTerms.splice(targetIndex, 0, draggedTerm); - saveTermOrder(newTerms); return newTerms; }); setDraggedTerm(null); };Then persist in a
useEffectthat watchesterms:useEffect(() => { if (terms.length > 0) { saveTermOrder(terms); } }, [terms]);
50-57: Same issue: side effect insetTermsupdater athandleRestoreTerm.
saveTermOrder(updated)on line 55 is also inside a state updater. The sameuseEffect-based persistence approach from the previous comment would eliminate both.
182-232: Hidden terms section is always visible, even when no terms exist at all.When a user has never used the explain feature, both
termsandhiddenTermswill be empty, yet this collapsible section still renders showing "0 hidden terms." Consider hiding it when!hasTerms && !hasHiddenTerms.♻️ Proposed fix
{/* Explained Terms Section */} + {(hasTerms || hasHiddenTerms) && ( <div className="mt-6"> <button ...> ... </div> + )}
106-111: Template literal forcardStylesincludes leading/trailing whitespace and newlines.While Tailwind typically handles this fine, a cleaner approach is to use a single-line string or a utility like
clsx/cnto avoid accidental whitespace in the class list.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/components/dashboard/ExplainedTermsCard.tsx`:
- Around line 206-208: The hidden terms map uses key={term} which can collide
for duplicate values; update the map over hiddenTerms in ExplainedTermsCard (the
JSX block rendering hiddenTerms) to include the index parameter and set the key
to a stable combined string like `${term}-${index}` (matching the visible-terms
approach) so each rendered <div> has a unique key.
In `@frontend/messages/pl.json`:
- Around line 253-254: The Polish locale currently duplicates "przez" because
"builtWith" is "Zbudowane przez" while "byCommunity" remains "przez
społeczność."; update the "byCommunity" value to remove the leading "przez"
(e.g., change the value of the byCommunity key to "społeczność." or match
punctuation style used elsewhere) so the concatenation of builtWith +
byCommunity renders correctly; modify the JSON entry for the byCommunity key
accordingly.
🧹 Nitpick comments (1)
frontend/components/dashboard/ExplainedTermsCard.tsx (1)
68-84:draggedIndexis read from the outer closure inside a functional state updater.
setTerms(prevTerms => ...)correctly usesprevTermsfor the array, butdraggedIndexis captured from the enclosing scope. If React batches state updates,draggedIndexcould theoretically be stale relative toprevTerms. SincehandleDragStartandhandleDropare sequential user events, this works in practice, but using a ref fordraggedIndexwould be more robust.
| {hiddenTerms.map(term => ( | ||
| <div | ||
| key={term} |
There was a problem hiding this comment.
Hidden terms list uses key={term} — inconsistent with the visible-terms fix.
Visible terms at line 138 correctly use key={\${term}-${index}`}to avoid duplicate-key issues, but hidden terms here still usekey={term}`. Apply the same pattern for consistency and to prevent React key collisions if duplicate terms exist.
Proposed fix
- {hiddenTerms.map(term => (
+ {hiddenTerms.map((term, index) => (
<div
- key={term}
+ key={`${term}-${index}`}
className="group relative ..."📝 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.
| {hiddenTerms.map(term => ( | |
| <div | |
| key={term} | |
| {hiddenTerms.map((term, index) => ( | |
| <div | |
| key={`${term}-${index}`} |
🤖 Prompt for AI Agents
In `@frontend/components/dashboard/ExplainedTermsCard.tsx` around lines 206 - 208,
The hidden terms map uses key={term} which can collide for duplicate values;
update the map over hiddenTerms in ExplainedTermsCard (the JSX block rendering
hiddenTerms) to include the index parameter and set the key to a stable combined
string like `${term}-${index}` (matching the visible-terms approach) so each
rendered <div> has a unique key.
| "builtWith": "Zbudowane przez", | ||
| "byCommunity": "przez społeczność.", |
There was a problem hiding this comment.
Double "przez" — byCommunity needs updating to match the new builtWith.
With builtWith now set to "Zbudowane przez" and byCommunity still "przez społeczność.", the rendered text will read "Zbudowane przez przez społeczność." ("Built by by community.").
The English locale uses "Built by" + "community.", and the Ukrainian uses "Створено" + "спільнотою." — both avoid duplication. Update byCommunity here to drop the leading "przez":
Proposed fix
"builtWith": "Zbudowane przez",
- "byCommunity": "przez społeczność.",
+ "byCommunity": "społeczność.",📝 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.
| "builtWith": "Zbudowane przez", | |
| "byCommunity": "przez społeczność.", | |
| "builtWith": "Zbudowane przez", | |
| "byCommunity": "społeczność.", |
🤖 Prompt for AI Agents
In `@frontend/messages/pl.json` around lines 253 - 254, The Polish locale
currently duplicates "przez" because "builtWith" is "Zbudowane przez" while
"byCommunity" remains "przez społeczność."; update the "byCommunity" value to
remove the leading "przez" (e.g., change the value of the byCommunity key to
"społeczność." or match punctuation style used elsewhere) so the concatenation
of builtWith + byCommunity renders correctly; modify the JSON entry for the
byCommunity key accordingly.
2 participants
Summary by CodeRabbit