Conversation
Deploying coc-members with
|
| Latest commit: |
1f718da
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1af86915.coc-members.pages.dev |
| Branch Preview URL: | https://feat-responsiveness.coc-members.pages.dev |
📝 WalkthroughWalkthroughThis pull request refactors the responsive UI and styling across three DSA dashboard components: QuestionView, TopicView, and DsaDashboard. Changes include adjusted section padding, restructured card layouts with enhanced visual polish, improved responsive spacing across screen sizes, and refined typography and color schemes to align with updated design tokens. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@frontend/src/components/dsa/questions/QuestionView.jsx`:
- Around line 129-131: The parent container using className "space-y-5
sm:space-y-6 lg:space-y-8" and each child div (inside filteredQuestions.map)
using "mb-6 sm:mb-8 lg:mb-12" cause doubled vertical gaps; choose one
method—remove the per-card margins on the child div (the div rendered in
filteredQuestions.map with key={question.id}) so spacing is controlled solely by
the parent's space-y-* classes, i.e., delete the mb-* utilities from that child
element's className.
- Around line 73-81: The topic tag is rendered as a non-interactive <button>
(the element containing selectedTopic.title in QuestionView.jsx) but has no
onClick; change that <button> to a presentational element like <span> or <div>
(keeping the same className and styling such as border-3, bg-[`#C1502E`],
font-extrabold, shadow etc.), remove any button-specific attributes, and if the
element actually needs to be interactive instead add a proper onClick handler
and keyboard handling; ensure the replacement still renders selectedTopic.title.
- Around line 190-202: The button in QuestionView.jsx (the clickable element
rendering <ExternalLink /> inside QuestionView) lacks an accessible label;
update the button element to include a descriptive aria-label (e.g.,
aria-label={`Open question: ${question.title || 'link'}`}) or add
visually-hidden text inside the button that conveys “Open” for screen readers,
ensuring the existing decorative <ExternalLink /> and the tooltip <span> remain
unchanged; target the button element in the QuestionView component to implement
this change.
In `@frontend/src/components/dsa/topics/TopicView.jsx`:
- Line 75: The div in TopicView.jsx uses an external texture URL in the Tailwind
class bg-[url('https://www.transparenttextures.com/patterns/paper-fibers.png')],
which creates a third-party runtime dependency; replace that external URL with a
self-hosted asset (e.g., copy paper-fibers.png into your public/ or assets/
directory) and update the Tailwind class to point to the local path (e.g.,
bg-[url('/textures/paper-fibers.png')]) so the component (the absolute inset-0
opacity-10 ... div) loads the image from your own host instead of
transparenttextures.com.
🧹 Nitpick comments (3)
frontend/src/components/dsa/questions/QuestionView.jsx (1)
174-188:toggle.isPendingdisables all "Mark Done" buttons simultaneously.The
disabledcheck on Line 176 uses the mutation's globalisPendingflag. If a user toggles one question, every other question's button becomes disabled until the mutation resolves. Consider tracking the pending question ID to disable only the relevant button.Proposed fix sketch
- disabled={toggle.isPending} - className={`flex items-center gap-2 sm:gap-3 font-extrabold text-[`#2C1810`] dark:text-[`#F5E6D3`] ${toggle.isPending ? "opacity-60 cursor-not-allowed" : "" + disabled={toggle.isPending && toggle.variables === question.id} + className={`flex items-center gap-2 sm:gap-3 font-extrabold text-[`#2C1810`] dark:text-[`#F5E6D3`] ${toggle.isPending && toggle.variables === question.id ? "opacity-60 cursor-not-allowed" : ""frontend/src/components/dsa/topics/TopicView.jsx (1)
59-59:IconComponentis the same constant for every topic — hoist it outside.map().
const IconComponent = CodeXml;is re-assigned identically on every iteration. Move it above the.map()(or just useCodeXmldirectly in JSX) to make the intent clearer.Proposed fix
+ const IconComponent = CodeXml; <div className="flex flex-col gap-6 sm:gap-8 lg:gap-12 max-w-6xl mx-auto"> {filteredTopics.map((topic, index) => { - const IconComponent = CodeXml; const isEven = index % 2 === 0;frontend/src/pages/DsaDashboard.jsx (1)
22-23:sm:justify-betweenhas no visible effect with a single flex child.The header flex container only has one child (the logo box).
justify-betweenbehaves identically tojustify-center(orjustify-start) when there's a single item. If a second header element (e.g., theme toggle) is planned, this is fine as forward scaffolding — otherwise it's dead CSS.
| <div className="relative w-full sm:w-auto sm:self-start"> | ||
| <div | ||
| aria-hidden="true" | ||
| className="absolute inset-0 translate-x-2 translate-y-2 bg-[#2C1810] dark:bg-[#F5E6D3]" | ||
| className="absolute inset-0 translate-x-1.5 translate-y-1.5 sm:translate-x-2 sm:translate-y-2 bg-[#2C1810] dark:bg-[#F5E6D3]" | ||
| /> | ||
| <button className="relative border-4 border-black bg-[#C1502E] text-white px-6 py-3 font-extrabold shadow-[6px_6px_0_0_rgba(0,0,0,1)]"> | ||
| <button className="relative w-full sm:w-auto border-3 sm:border-4 border-black bg-[#C1502E] text-white px-5 py-2.5 sm:px-6 sm:py-3 font-extrabold shadow-[4px_4px_0_0_rgba(0,0,0,1)] sm:shadow-[6px_6px_0_0_rgba(0,0,0,1)] text-sm sm:text-base"> | ||
| {selectedTopic.title} | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Non-interactive element rendered as <button>.
The topic tag (Line 78) is a <button> with no onClick handler — it's purely presentational. This misleads assistive technology into announcing it as an interactive control. Use a <span> or <div> instead, and keep the same styling classes.
Proposed fix
- <button className="relative w-full sm:w-auto border-3 sm:border-4 border-black bg-[`#C1502E`] text-white px-5 py-2.5 sm:px-6 sm:py-3 font-extrabold shadow-[4px_4px_0_0_rgba(0,0,0,1)] sm:shadow-[6px_6px_0_0_rgba(0,0,0,1)] text-sm sm:text-base">
+ <span className="relative inline-block w-full sm:w-auto border-3 sm:border-4 border-black bg-[`#C1502E`] text-white px-5 py-2.5 sm:px-6 sm:py-3 font-extrabold shadow-[4px_4px_0_0_rgba(0,0,0,1)] sm:shadow-[6px_6px_0_0_rgba(0,0,0,1)] text-sm sm:text-base">
{selectedTopic.title}
- </button>
+ </span>📝 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.
| <div className="relative w-full sm:w-auto sm:self-start"> | |
| <div | |
| aria-hidden="true" | |
| className="absolute inset-0 translate-x-2 translate-y-2 bg-[#2C1810] dark:bg-[#F5E6D3]" | |
| className="absolute inset-0 translate-x-1.5 translate-y-1.5 sm:translate-x-2 sm:translate-y-2 bg-[#2C1810] dark:bg-[#F5E6D3]" | |
| /> | |
| <button className="relative border-4 border-black bg-[#C1502E] text-white px-6 py-3 font-extrabold shadow-[6px_6px_0_0_rgba(0,0,0,1)]"> | |
| <button className="relative w-full sm:w-auto border-3 sm:border-4 border-black bg-[#C1502E] text-white px-5 py-2.5 sm:px-6 sm:py-3 font-extrabold shadow-[4px_4px_0_0_rgba(0,0,0,1)] sm:shadow-[6px_6px_0_0_rgba(0,0,0,1)] text-sm sm:text-base"> | |
| {selectedTopic.title} | |
| </button> | |
| </div> | |
| <div className="relative w-full sm:w-auto sm:self-start"> | |
| <div | |
| aria-hidden="true" | |
| className="absolute inset-0 translate-x-1.5 translate-y-1.5 sm:translate-x-2 sm:translate-y-2 bg-[`#2C1810`] dark:bg-[`#F5E6D3`]" | |
| /> | |
| <span className="relative inline-block w-full sm:w-auto border-3 sm:border-4 border-black bg-[`#C1502E`] text-white px-5 py-2.5 sm:px-6 sm:py-3 font-extrabold shadow-[4px_4px_0_0_rgba(0,0,0,1)] sm:shadow-[6px_6px_0_0_rgba(0,0,0,1)] text-sm sm:text-base"> | |
| {selectedTopic.title} | |
| </span> | |
| </div> |
🤖 Prompt for AI Agents
In `@frontend/src/components/dsa/questions/QuestionView.jsx` around lines 73 - 81,
The topic tag is rendered as a non-interactive <button> (the element containing
selectedTopic.title in QuestionView.jsx) but has no onClick; change that
<button> to a presentational element like <span> or <div> (keeping the same
className and styling such as border-3, bg-[`#C1502E`], font-extrabold, shadow
etc.), remove any button-specific attributes, and if the element actually needs
to be interactive instead add a proper onClick handler and keyboard handling;
ensure the replacement still renders selectedTopic.title.
| <div className="space-y-5 sm:space-y-6 lg:space-y-8"> | ||
| {filteredQuestions.map((question) => ( | ||
| <div key={question.id} className="relative group mb-12"> | ||
| {/* Layered block background (non-interactive) */} | ||
| <div | ||
| className="absolute inset-0 -rotate-1 translate-x-3 translate-y-3 bg-[#2C1810] dark:bg-[#F5E6D3] rounded-lg transition-all duration-500 group-hover:translate-x-4 group-hover:translate-y-4 pointer-events-none" | ||
| /> | ||
| <div | ||
| className="absolute inset-0 rotate-2 translate-x-1 translate-y-1 bg-[#b05a3c]/60 rounded-lg blur-[1px] pointer-events-none" | ||
| /> | ||
|
|
||
| {/* Main floating card */} | ||
| <div | ||
| className="relative border-4 border-black bg-[#FFF6EE] dark:bg-[#2C1810] | ||
| p-8 rounded-lg shadow-[10px_10px_0_0_#2C1810] | ||
| hover:shadow-[12px_12px_0_0_#C1502E] | ||
| transition-all duration-300 hover:-translate-y-2" | ||
| > | ||
| {/* Header */} | ||
| <div className="flex items-start justify-between gap-6"> | ||
| <div className="flex-1"> | ||
| <h3 | ||
| className={`text-2xl font-extrabold leading-snug ${ | ||
| completedMap[question.id] | ||
| ? "text-[#a88d80] line-through" | ||
| : "text-[#2C1810] dark:text-[#F5E6D3]" | ||
| }`} | ||
| > | ||
| {question.questionName} | ||
| </h3> | ||
| </div> | ||
|
|
||
| <div | ||
| className={`text-sm font-black tracking-wider border-4 border-black | ||
| px-4 py-1 bg-[#C1502E] text-[#FFF6EE] dark:bg-[#F5E6D3] dark:text-[#2C1810] | ||
| shadow-[3px_3px_0_0_#2C1810]`} | ||
| > | ||
| {question.difficulty.toUpperCase()} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Divider bar */} | ||
| <div className="h-1 mt-5 mb-6 bg-[#C1502E] dark:bg-[#F5E6D3] rounded-full w-2/3 group-hover:w-full transition-all duration-300"></div> | ||
|
|
||
| {/* Actions */} | ||
| <div className="flex justify-between items-center"> | ||
| <button | ||
| onClick={() => handleToggle(question.id)} | ||
| disabled={toggle.isPending} | ||
| className={`flex items-center gap-3 font-extrabold text-[#2C1810] dark:text-[#F5E6D3] ${ | ||
| toggle.isPending ? "opacity-60 cursor-not-allowed" : "" | ||
| }`} | ||
| > | ||
| {completedMap[question.id] ? ( | ||
| <CheckCircle2 className="w-8 h-8 text-[#3dd68c]" /> | ||
| ) : ( | ||
| <Circle className="w-8 h-8 text-[#b05a3c] dark:text-[#F5E6D3]" /> | ||
| )} | ||
| <span className="text-sm uppercase"> | ||
| {toggle.isPending ? "Updating..." : "Mark Done"} | ||
| </span> | ||
| </button> | ||
|
|
||
| <button | ||
| onClick={() => | ||
| window.open(question.link, "_blank", "noopener,noreferrer") | ||
| } | ||
| className="relative border-4 border-black bg-[#F5E6D3] dark:bg-[#FFF6EE] | ||
| p-3 font-black hover:-translate-x-0.5 hover:-translate-y-0.5 | ||
| shadow-[4px_4px_0_0_#2C1810] transition-transform group/link" | ||
| > | ||
| <ExternalLink className="w-5 h-5 text-[#2C1810]" /> | ||
| <span className="absolute -bottom-5 left-0 opacity-0 group-hover/link:opacity-100 text-xs font-extrabold text-[#2C1810] dark:text-[#F5E6D3] transition-all"> | ||
| Open | ||
| </span> | ||
| </button> | ||
| </div> | ||
|
|
||
| {/* Floating animated accent */} | ||
| <div className="absolute -bottom-2 right-6 w-20 h-20 bg-gradient-to-tr from-[#C1502E40] to-transparent blur-2xl rounded-full animate-pulse pointer-events-none" /> | ||
| </div> | ||
| </div> | ||
| ))} | ||
| <div key={question.id} className="relative group mb-6 sm:mb-8 lg:mb-12"> |
There was a problem hiding this comment.
Double vertical spacing — space-y-* on parent compounds with mb-* on children.
Line 129 applies space-y-5 sm:space-y-6 lg:space-y-8 to the container, which adds margin-top to every sibling. Line 131 also adds mb-6 sm:mb-8 lg:mb-12 to each card. This stacks both margins between cards, producing larger gaps than intended.
Pick one approach — either space-y-* on the parent or mb-* on the children — not both.
Proposed fix — remove mb from children
- <div key={question.id} className="relative group mb-6 sm:mb-8 lg:mb-12">
+ <div key={question.id} className="relative group">📝 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.
| <div className="space-y-5 sm:space-y-6 lg:space-y-8"> | |
| {filteredQuestions.map((question) => ( | |
| <div key={question.id} className="relative group mb-12"> | |
| {/* Layered block background (non-interactive) */} | |
| <div | |
| className="absolute inset-0 -rotate-1 translate-x-3 translate-y-3 bg-[#2C1810] dark:bg-[#F5E6D3] rounded-lg transition-all duration-500 group-hover:translate-x-4 group-hover:translate-y-4 pointer-events-none" | |
| /> | |
| <div | |
| className="absolute inset-0 rotate-2 translate-x-1 translate-y-1 bg-[#b05a3c]/60 rounded-lg blur-[1px] pointer-events-none" | |
| /> | |
| {/* Main floating card */} | |
| <div | |
| className="relative border-4 border-black bg-[#FFF6EE] dark:bg-[#2C1810] | |
| p-8 rounded-lg shadow-[10px_10px_0_0_#2C1810] | |
| hover:shadow-[12px_12px_0_0_#C1502E] | |
| transition-all duration-300 hover:-translate-y-2" | |
| > | |
| {/* Header */} | |
| <div className="flex items-start justify-between gap-6"> | |
| <div className="flex-1"> | |
| <h3 | |
| className={`text-2xl font-extrabold leading-snug ${ | |
| completedMap[question.id] | |
| ? "text-[#a88d80] line-through" | |
| : "text-[#2C1810] dark:text-[#F5E6D3]" | |
| }`} | |
| > | |
| {question.questionName} | |
| </h3> | |
| </div> | |
| <div | |
| className={`text-sm font-black tracking-wider border-4 border-black | |
| px-4 py-1 bg-[#C1502E] text-[#FFF6EE] dark:bg-[#F5E6D3] dark:text-[#2C1810] | |
| shadow-[3px_3px_0_0_#2C1810]`} | |
| > | |
| {question.difficulty.toUpperCase()} | |
| </div> | |
| </div> | |
| {/* Divider bar */} | |
| <div className="h-1 mt-5 mb-6 bg-[#C1502E] dark:bg-[#F5E6D3] rounded-full w-2/3 group-hover:w-full transition-all duration-300"></div> | |
| {/* Actions */} | |
| <div className="flex justify-between items-center"> | |
| <button | |
| onClick={() => handleToggle(question.id)} | |
| disabled={toggle.isPending} | |
| className={`flex items-center gap-3 font-extrabold text-[#2C1810] dark:text-[#F5E6D3] ${ | |
| toggle.isPending ? "opacity-60 cursor-not-allowed" : "" | |
| }`} | |
| > | |
| {completedMap[question.id] ? ( | |
| <CheckCircle2 className="w-8 h-8 text-[#3dd68c]" /> | |
| ) : ( | |
| <Circle className="w-8 h-8 text-[#b05a3c] dark:text-[#F5E6D3]" /> | |
| )} | |
| <span className="text-sm uppercase"> | |
| {toggle.isPending ? "Updating..." : "Mark Done"} | |
| </span> | |
| </button> | |
| <button | |
| onClick={() => | |
| window.open(question.link, "_blank", "noopener,noreferrer") | |
| } | |
| className="relative border-4 border-black bg-[#F5E6D3] dark:bg-[#FFF6EE] | |
| p-3 font-black hover:-translate-x-0.5 hover:-translate-y-0.5 | |
| shadow-[4px_4px_0_0_#2C1810] transition-transform group/link" | |
| > | |
| <ExternalLink className="w-5 h-5 text-[#2C1810]" /> | |
| <span className="absolute -bottom-5 left-0 opacity-0 group-hover/link:opacity-100 text-xs font-extrabold text-[#2C1810] dark:text-[#F5E6D3] transition-all"> | |
| Open | |
| </span> | |
| </button> | |
| </div> | |
| {/* Floating animated accent */} | |
| <div className="absolute -bottom-2 right-6 w-20 h-20 bg-gradient-to-tr from-[#C1502E40] to-transparent blur-2xl rounded-full animate-pulse pointer-events-none" /> | |
| </div> | |
| </div> | |
| ))} | |
| <div key={question.id} className="relative group mb-6 sm:mb-8 lg:mb-12"> | |
| <div className="space-y-5 sm:space-y-6 lg:space-y-8"> | |
| {filteredQuestions.map((question) => ( | |
| <div key={question.id} className="relative group"> |
🤖 Prompt for AI Agents
In `@frontend/src/components/dsa/questions/QuestionView.jsx` around lines 129 -
131, The parent container using className "space-y-5 sm:space-y-6 lg:space-y-8"
and each child div (inside filteredQuestions.map) using "mb-6 sm:mb-8 lg:mb-12"
cause doubled vertical gaps; choose one method—remove the per-card margins on
the child div (the div rendered in filteredQuestions.map with key={question.id})
so spacing is controlled solely by the parent's space-y-* classes, i.e., delete
the mb-* utilities from that child element's className.
| <button | ||
| onClick={() => | ||
| window.open(question.link, "_blank", "noopener,noreferrer") | ||
| } | ||
| className="relative border-3 sm:border-4 border-black bg-[#F5E6D3] dark:bg-[#FFF6EE] | ||
| p-2.5 sm:p-3 font-black hover:-translate-x-0.5 hover:-translate-y-0.5 | ||
| shadow-[3px_3px_0_0_#2C1810] sm:shadow-[4px_4px_0_0_#2C1810] transition-transform group/link" | ||
| > | ||
| <ExternalLink className="w-4 h-4 sm:w-5 sm:h-5 text-[#2C1810]" /> | ||
| <span className="absolute -bottom-4 sm:-bottom-5 left-0 opacity-0 group-hover/link:opacity-100 text-[10px] sm:text-xs font-extrabold text-[#2C1810] dark:text-[#F5E6D3] transition-all"> | ||
| Open | ||
| </span> | ||
| </button> |
There was a problem hiding this comment.
Missing accessible label on the "Open" link button.
The button only contains an <ExternalLink> icon and a tooltip <span> that's opacity-0 by default. Screen readers won't convey the button's purpose. Add an aria-label (or visually-hidden text).
Proposed fix
<button
onClick={() =>
window.open(question.link, "_blank", "noopener,noreferrer")
}
- className="relative border-3 sm:border-4 border-black bg-[`#F5E6D3`] dark:bg-[`#FFF6EE`]
+ aria-label="Open question in new tab"
+ className="relative border-3 sm:border-4 border-black bg-[`#F5E6D3`] dark:bg-[`#FFF6EE`]
p-2.5 sm:p-3 font-black hover:-translate-x-0.5 hover:-translate-y-0.5
shadow-[3px_3px_0_0_#2C1810] sm:shadow-[4px_4px_0_0_#2C1810] transition-transform group/link"
>🤖 Prompt for AI Agents
In `@frontend/src/components/dsa/questions/QuestionView.jsx` around lines 190 -
202, The button in QuestionView.jsx (the clickable element rendering
<ExternalLink /> inside QuestionView) lacks an accessible label; update the
button element to include a descriptive aria-label (e.g., aria-label={`Open
question: ${question.title || 'link'}`}) or add visually-hidden text inside the
button that conveys “Open” for screen readers, ensuring the existing decorative
<ExternalLink /> and the tooltip <span> remain unchanged; target the button
element in the QuestionView component to implement this change.
| <div className="absolute inset-0 opacity-10 dark:opacity-20 mix-blend-multiply pointer-events-none bg-[url('https://www.transparenttextures.com/patterns/paper-fibers.png')] bg-repeat"></div> | ||
| > | ||
| {/* Subtle paper texture */} | ||
| <div className="absolute inset-0 opacity-10 dark:opacity-20 mix-blend-multiply pointer-events-none bg-[url('https://www.transparenttextures.com/patterns/paper-fibers.png')] bg-repeat"></div> |
There was a problem hiding this comment.
External texture URL introduces a third-party runtime dependency.
bg-[url('https://www.transparenttextures.com/patterns/paper-fibers.png')] fetches an image from a third-party host on every page load. If that site is slow or unreachable the request hangs or fails silently; it also leaks visitor IPs to a third party. Consider self-hosting the texture in your public/ or assets/ directory.
🤖 Prompt for AI Agents
In `@frontend/src/components/dsa/topics/TopicView.jsx` at line 75, The div in
TopicView.jsx uses an external texture URL in the Tailwind class
bg-[url('https://www.transparenttextures.com/patterns/paper-fibers.png')], which
creates a third-party runtime dependency; replace that external URL with a
self-hosted asset (e.g., copy paper-fibers.png into your public/ or assets/
directory) and update the Tailwind class to point to the local path (e.g.,
bg-[url('/textures/paper-fibers.png')]) so the component (the absolute inset-0
opacity-10 ... div) loads the image from your own host instead of
transparenttextures.com.
Summary by CodeRabbit
Style