Skip to content

Back button fix within wave and modal width full#1636

Merged
ragnep merged 8 commits intomainfrom
ux-fixes-back-button-and-modal
Dec 6, 2025
Merged

Back button fix within wave and modal width full#1636
ragnep merged 8 commits intomainfrom
ux-fixes-back-button-and-modal

Conversation

@ragnep
Copy link
Copy Markdown
Contributor

@ragnep ragnep commented Dec 6, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Preserve and restore the correct active view based on route and transient "wave" states, avoiding unexpected switches to default.
  • Refactor

    • Centralized and simplified view-restoration logic for more predictable navigation and route-aware behavior.
  • Style

    • Voting modal now renders via a portal with improved spacing, backdrop behavior, and layering.
  • Accessibility

    • Modal gains focus management, keyboard handling (Escape to close), scroll prevention while open, and improved ARIA labeling.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: ragnep <ragneinfo@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 6, 2025

Walkthrough

Updates mobile view restoration and guards based on route and wave presence in BrainMobile; VotingModal now mounts via a portal with focus management, backdrop/escape handling, and body-scroll prevention; ModalLayout gains an optional titleId prop and applies it to the rendered title for ARIA linkage.

Changes

Cohort / File(s) Summary
Route-aware view restoration
components/brain/BrainMobile.tsx
Added globalViews and routeToView mapping; included pathname in effect dependencies; consolidated per-view resets into shouldResetToDefault; restore activeView from route or DEFAULT when no active wave; reset to DEFAULT for certain non-wave views when a wave is present.
Modal portal, accessibility & focus
components/voting/VotingModal.tsx
Modal now renders via createPortal; extracted modal content; added focus management (ref + restore), Escape key handling, focus trapping, role="dialog", aria-modal, aria-labelledby, tabIndex={-1}; backdrop click closes modal, inner content stops propagation; disables body scroll while open; added horizontal padding.
ARIA prop for modal layout
components/waves/memes/submission/layout/ModalLayout.tsx
Added optional titleId?: string to ModalLayoutProps and component props; applied id={titleId} to title element to enable ARIA labelling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect BrainMobile effect logic for correct dependency handling and route-to-view mapping.
  • Verify VotingModal mount/unmount via portal, focus management, Escape/backdrop behavior, and body-scroll toggling.
  • Confirm ModalLayout titleId propagation and that callers render accessible titles.

Suggested reviewers

  • simo6529

"I hopped and patched with nimble paws,
Paths tune views and portals pause,
A padded nest where voters stay,
Titles sing for screen-reader play,
Rabbit nods — the updates prance and cause." 🐰

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title mentions 'back button fix' and 'modal width full', but the actual changes focus on view state management, focus/keyboard handling, accessibility improvements, and portal rendering. The title does not accurately reflect the substantial changes made. Revise the title to reflect the main changes, such as 'Improve modal accessibility and fix view state management' or similar, which better captures the actual implementation.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ux-fixes-back-button-and-modal

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
components/brain/BrainMobile.tsx (1)

163-181: Add pathname to the effect dependencies and drop the new inline comment

The new route‑aware reset logic when !hasWave looks correct and should address the back‑navigation bug, but there are two follow‑ups:

  1. pathname is now used inside this effect but is missing from the dependency array, which will trip react-hooks/exhaustive-deps and can yield stale behavior if the path changes without other deps:
-  }, [hasWave, wave, isCompleted, firstDecisionDone, activeView, isMemesWave, waveId]);
+  }, [hasWave, wave, isCompleted, firstDecisionDone, activeView, isMemesWave, waveId, pathname]);
  1. Per the repo guidelines for TSX/JSX, avoid inline comments in code. This comment is redundant with the condition and can be removed:
-      ) {
-        // Reset to appropriate view based on current route
-        if (pathname === "/waves") {
+      ) {
+        if (pathname === "/waves") {
           setActiveView(BrainView.WAVES);
         } else if (pathname === "/messages") {
           setActiveView(BrainView.MESSAGES);
         } else {
           setActiveView(BrainView.DEFAULT);
         }
       }
🧹 Nitpick comments (1)
components/voting/VotingModal.tsx (1)

3-52: Portal refactor looks good; just confirm client‑only usage of document.body

The change to render the modal via createPortal into document.body with a higher z‑index and extra horizontal padding is clean and should resolve stacking/overlay issues; the structure and click handling remain correct.

One thing to double‑check: because this component now references document.body during render, it must only ever be used from Client Components / browser contexts. If there is any chance it’s rendered from a Server Component or during SSR outside a client boundary, you’ll want to either:

  • Ensure all call sites are within 'use client' components, or
  • Add a small guard before returning the portal, e.g.:
if (typeof document === "undefined") {
  return null;
}

return createPortal(modalContent, document.body);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91d0a29 and ff8faef.

📒 Files selected for processing (2)
  • components/brain/BrainMobile.tsx (1 hunks)
  • components/voting/VotingModal.tsx (4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version

**/*.{ts,tsx,js,jsx}: Replace <img> elements with <Image /> from next/image to satisfy @next/next/no-img-element ESLint rule
Use <Link href="/path"> from Next.js for internal navigation instead of plain HTML links to satisfy @next/next/no-html-link-for-pages ESLint rule

Files:

  • components/voting/VotingModal.tsx
  • components/brain/BrainMobile.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always add readonly before props in React components

Files:

  • components/voting/VotingModal.tsx
  • components/brain/BrainMobile.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use <Link>, images should use next/image, and adopt Next's ESLint rules (Core Web Vitals)

**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks rules)
Follow existing code style and naming conventions; maintain clean code standards (measured by SonarQube)

Files:

  • components/voting/VotingModal.tsx
  • components/brain/BrainMobile.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Must pass tsc --noEmit type checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") over React. namespace usage (React.useMemo, React.useRef, etc.)
If the react-hooks/exhaustive-deps lint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic in useEffectEvent

**/*.{ts,tsx}: Must pass tsc --noEmit for TypeScript type checking
Prefer Server Components over Client Components; use Server Functions/Server Actions ('use server') for mutations
Remove unnecessary Effects; if Effect only derives state, compute during render instead
Use useEffectEvent for non-reactive logic inside Effects to avoid unnecessary re-runs
Use framework APIs: <Link> for internal links, next/image for images, adopt Next's ESLint rules
Use 'use cache' directive and Cache Components features for explicit opt-in caching in Next.js 16
Use TypeScript and React functional components with hooks
When parsing Seize URLs or similar, fail fast if base origin is unavailable; do not fall back to placeholder origins
Replace <img> elements with <Image /> from next/image
Use <Link href="/path"> for internal navigation instead of plain HTML links
Move data fetches to Server Components; handle mutations through Server Functions/Server Actions with 'use server' directive

Files:

  • components/voting/VotingModal.tsx
  • components/brain/BrainMobile.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.255Z
Learning: Fix with modernization (no `// eslint-disable` unless explicitly instructed); prefer refactors aligned with React 19.2, React Compiler, and Next.js 16 conventions
📚 Learning: 2025-12-03T14:52:34.255Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.255Z
Learning: Fix with modernization (no `// eslint-disable` unless explicitly instructed); prefer refactors aligned with React 19.2, React Compiler, and Next.js 16 conventions

Applied to files:

  • components/voting/VotingModal.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
components/voting/VotingModal.tsx (1)

34-47: Add essential accessibility features for the modal.

The modal is missing critical accessibility attributes and behaviors:

  1. ARIA attributes: Add role="dialog", aria-modal="true", and aria-labelledby to the modal container
  2. Keyboard support: Implement Escape key handler to close the modal
  3. Focus management: Move focus to the modal when it opens and restore focus when it closes
  4. Scroll lock: Prevent body scrolling while the modal is open

These features are essential for keyboard navigation and screen reader users to interact with the modal effectively.

Would you like me to generate a comprehensive solution that includes these accessibility improvements, possibly using a library like react-focus-lock for focus management or implementing a custom solution?

🧹 Nitpick comments (3)
components/voting/VotingModal.tsx (1)

24-27: Remove redundant stopPropagation on outer container.

The stopPropagation on line 26 is redundant since this is the outermost element. The effective pattern is:

  • Backdrop (lines 28-32) closes the modal when clicked
  • Modal content (line 36) stops propagation to prevent triggering the backdrop handler

The outer container's stopPropagation serves no purpose and can be removed for clarity.

 const modalContent = (
   <div
     className="tw-fixed tw-inset-0 tw-bg-gray-600 tw-bg-opacity-50 tw-backdrop-blur-[1px] tw-z-50 tw-flex tw-items-center tw-justify-center"
-    onClick={(e) => e.stopPropagation()}
   >
components/brain/BrainMobile.tsx (2)

172-175: Consider adding /notifications to the routeToView mapping for consistency.

While the first useEffect (lines 129-135) explicitly handles /notifications pathname, the routeToView mapping serves as a fallback for restoring views when a wave is dismissed. Including /notifications would make the mapping complete and handle edge cases more gracefully.

 const routeToView: Record<string, BrainView> = {
   "/waves": BrainView.WAVES,
   "/messages": BrainView.MESSAGES,
+  "/notifications": BrainView.NOTIFICATIONS,
 };

165-175: Consider extracting static constants outside the component for better performance.

The globalViews and routeToView constants (and nonWaveViews at lines 198-202) are recreated on every useEffect run. Since they don't depend on props or state, extracting them as module-level constants would improve performance and clarity.

// Add before the component
const GLOBAL_VIEWS = new Set([
  BrainView.DEFAULT,
  BrainView.WAVES,
  BrainView.MESSAGES,
  BrainView.NOTIFICATIONS,
]);

const ROUTE_TO_VIEW: Record<string, BrainView> = {
  "/waves": BrainView.WAVES,
  "/messages": BrainView.MESSAGES,
  "/notifications": BrainView.NOTIFICATIONS,
};

const NON_WAVE_VIEWS = new Set([
  BrainView.NOTIFICATIONS,
  BrainView.MESSAGES,
  BrainView.WAVES,
]);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff8faef and ed10a40.

📒 Files selected for processing (2)
  • components/brain/BrainMobile.tsx (1 hunks)
  • components/voting/VotingModal.tsx (4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version

**/*.{ts,tsx,js,jsx}: Replace <img> elements with <Image /> from next/image to satisfy @next/next/no-img-element ESLint rule
Use <Link href="/path"> from Next.js for internal navigation instead of plain HTML links to satisfy @next/next/no-html-link-for-pages ESLint rule

Files:

  • components/brain/BrainMobile.tsx
  • components/voting/VotingModal.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always add readonly before props in React components

Files:

  • components/brain/BrainMobile.tsx
  • components/voting/VotingModal.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use <Link>, images should use next/image, and adopt Next's ESLint rules (Core Web Vitals)

**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks rules)
Follow existing code style and naming conventions; maintain clean code standards (measured by SonarQube)

Files:

  • components/brain/BrainMobile.tsx
  • components/voting/VotingModal.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Must pass tsc --noEmit type checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") over React. namespace usage (React.useMemo, React.useRef, etc.)
If the react-hooks/exhaustive-deps lint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic in useEffectEvent

**/*.{ts,tsx}: Must pass tsc --noEmit for TypeScript type checking
Prefer Server Components over Client Components; use Server Functions/Server Actions ('use server') for mutations
Remove unnecessary Effects; if Effect only derives state, compute during render instead
Use useEffectEvent for non-reactive logic inside Effects to avoid unnecessary re-runs
Use framework APIs: <Link> for internal links, next/image for images, adopt Next's ESLint rules
Use 'use cache' directive and Cache Components features for explicit opt-in caching in Next.js 16
Use TypeScript and React functional components with hooks
When parsing Seize URLs or similar, fail fast if base origin is unavailable; do not fall back to placeholder origins
Replace <img> elements with <Image /> from next/image
Use <Link href="/path"> for internal navigation instead of plain HTML links
Move data fetches to Server Components; handle mutations through Server Functions/Server Actions with 'use server' directive

Files:

  • components/brain/BrainMobile.tsx
  • components/voting/VotingModal.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-03T14:52:34.255Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.255Z
Learning: Fix with modernization (no `// eslint-disable` unless explicitly instructed); prefer refactors aligned with React 19.2, React Compiler, and Next.js 16 conventions

Applied to files:

  • components/brain/BrainMobile.tsx
  • components/voting/VotingModal.tsx
🔇 Additional comments (2)
components/voting/VotingModal.tsx (1)

23-52: LGTM! Portal implementation and styling improvements.

The changes effectively improve the modal implementation:

  • Extracting modalContent improves readability
  • Using createPortal is the correct pattern for rendering modals at the document root
  • Adding tw-px-4 provides appropriate horizontal spacing
components/brain/BrainMobile.tsx (1)

164-207: Nice refactoring that improves view restoration and back button handling.

The changes successfully consolidate view reset logic and add pathname-based restoration when waves are dismissed. The separation of concerns between URL-based view setting (first useEffect) and wave-state validation (this useEffect) is clear and maintainable.

Key improvements:

  • Route-aware view restoration prevents always defaulting to BrainView.DEFAULT
  • Single shouldResetToDefault guard simplifies multiple reset conditions
  • Properly handles non-wave views when a wave becomes active
  • Adding pathname to dependencies ensures route changes trigger appropriate restoration

Comment thread components/voting/VotingModal.tsx
Signed-off-by: ragnep <ragneinfo@gmail.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
components/brain/BrainMobile.tsx (1)

165-208: Wave/global view reset logic looks sound; consider hoisting constants for clarity

The new globalViews / routeToView handling for !hasWave correctly restores activeView from the current pathname when leaving a wave-specific view, and the consolidated shouldResetToDefault plus nonWaveViews guard behaves deterministically (no infinite loops, and dependencies are complete, including pathname). This should address the back-button/stale-view issue without regressions.

If you want to tighten things up further, you can hoist the static sets/maps out of the effect so they’re defined once and reused, which makes the intent clearer and avoids re-allocating them on every run:

+const GLOBAL_VIEWS = new Set<BrainView>([
+  BrainView.DEFAULT,
+  BrainView.WAVES,
+  BrainView.MESSAGES,
+  BrainView.NOTIFICATIONS,
+]);
+
+const ROUTE_TO_VIEW: Partial<Record<string, BrainView>> = {
+  "/waves": BrainView.WAVES,
+  "/messages": BrainView.MESSAGES,
+  "/notifications": BrainView.NOTIFICATIONS,
+};
+
+const NON_WAVE_VIEWS = new Set<BrainView>([
+  BrainView.NOTIFICATIONS,
+  BrainView.MESSAGES,
+  BrainView.WAVES,
+]);
+
 // Handle tab visibility and reset on wave changes
 useEffect(() => {
-  const globalViews = new Set([
-    BrainView.DEFAULT,
-    BrainView.WAVES,
-    BrainView.MESSAGES,
-    BrainView.NOTIFICATIONS,
-  ]);
-
-  const routeToView: Record<string, BrainView> = {
-    "/waves": BrainView.WAVES,
-    "/messages": BrainView.MESSAGES,
-    "/notifications": BrainView.NOTIFICATIONS,
-  };
+  const globalViews = GLOBAL_VIEWS;
+  const routeToView = ROUTE_TO_VIEW;
   ...
-  const nonWaveViews = new Set([
-    BrainView.NOTIFICATIONS,
-    BrainView.MESSAGES,
-    BrainView.WAVES,
-  ]);
+  const nonWaveViews = NON_WAVE_VIEWS;
   ...
 }, [hasWave, wave, isCompleted, firstDecisionDone, activeView, isMemesWave, waveId, pathname]);

This keeps behavior identical while centralizing the view/route definitions.

Please double-check that the string route keys ("/waves", "/messages", "/notifications") still match your actual pathnames (no base path or trailing slash), since both this effect and the earlier one rely on exact matches.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed10a40 and 83b8573.

📒 Files selected for processing (2)
  • components/brain/BrainMobile.tsx (1 hunks)
  • components/voting/VotingModal.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • components/voting/VotingModal.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version

**/*.{ts,tsx,js,jsx}: Replace <img> elements with <Image /> from next/image to satisfy @next/next/no-img-element ESLint rule
Use <Link href="/path"> from Next.js for internal navigation instead of plain HTML links to satisfy @next/next/no-html-link-for-pages ESLint rule

Files:

  • components/brain/BrainMobile.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always add readonly before props in React components

Files:

  • components/brain/BrainMobile.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use <Link>, images should use next/image, and adopt Next's ESLint rules (Core Web Vitals)

**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks rules)
Follow existing code style and naming conventions; maintain clean code standards (measured by SonarQube)

Files:

  • components/brain/BrainMobile.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Must pass tsc --noEmit type checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") over React. namespace usage (React.useMemo, React.useRef, etc.)
If the react-hooks/exhaustive-deps lint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic in useEffectEvent

**/*.{ts,tsx}: Must pass tsc --noEmit for TypeScript type checking
Prefer Server Components over Client Components; use Server Functions/Server Actions ('use server') for mutations
Remove unnecessary Effects; if Effect only derives state, compute during render instead
Use useEffectEvent for non-reactive logic inside Effects to avoid unnecessary re-runs
Use framework APIs: <Link> for internal links, next/image for images, adopt Next's ESLint rules
Use 'use cache' directive and Cache Components features for explicit opt-in caching in Next.js 16
Use TypeScript and React functional components with hooks
When parsing Seize URLs or similar, fail fast if base origin is unavailable; do not fall back to placeholder origins
Replace <img> elements with <Image /> from next/image
Use <Link href="/path"> for internal navigation instead of plain HTML links
Move data fetches to Server Components; handle mutations through Server Functions/Server Actions with 'use server' directive

Files:

  • components/brain/BrainMobile.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)

Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Dec 6, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
2 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@ragnep ragnep merged commit e044ccc into main Dec 6, 2025
7 of 8 checks passed
@ragnep ragnep deleted the ux-fixes-back-button-and-modal branch December 6, 2025 13:33
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
components/voting/VotingModal.tsx (2)

1-5: Client directive and React imports are correct; consider type-only FC import

The "use client" directive plus direct hook imports are appropriate here given the DOM access and portal usage. To align with the “named imports for types” guideline and avoid pulling the default React object at runtime, you can switch to a type-only FC import:

-"use client";
-
-import React, { useEffect, useId, useRef } from "react";
+"use client";
+
+import type { FC } from "react";
+import { useEffect, useId, useRef } from "react";
@@
-const VotingModal: React.FC<VotingModalProps> = ({
+const VotingModal: FC<VotingModalProps> = ({

21-47: Effect-based focus management and scroll lock look solid; add a small safety guard

The effect correctly: (1) stores the previously focused element, (2) focuses the dialog, (3) locks body scroll, and (4) restores both on close/unmount. To make focus restoration more robust (e.g., if the previously focused element unmounts while the modal is open), you can guard the focus call:

   useEffect(() => {
     if (!isOpen) return;
@@
-    return () => {
-      document.body.style.overflow = originalOverflow;
-      document.removeEventListener("keydown", handleKeyDown);
-      previousActiveElement.current?.focus();
-    };
+    return () => {
+      document.body.style.overflow = originalOverflow;
+      document.removeEventListener("keydown", handleKeyDown);
+      const element = previousActiveElement.current;
+      if (element && document.contains(element)) {
+        element.focus();
+      }
+    };
   }, [isOpen, onClose]);

This keeps the current behavior but avoids attempting to focus a detached node. Based on learnings, this is an appropriate use of a Client Component-side effect for DOM concerns.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d88ba0 and 2314c3a.

📒 Files selected for processing (1)
  • components/voting/VotingModal.tsx (4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version

**/*.{ts,tsx,js,jsx}: Replace <img> elements with <Image /> from next/image to satisfy @next/next/no-img-element ESLint rule
Use <Link href="/path"> from Next.js for internal navigation instead of plain HTML links to satisfy @next/next/no-html-link-for-pages ESLint rule

Files:

  • components/voting/VotingModal.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always add readonly before props in React components

Files:

  • components/voting/VotingModal.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use <Link>, images should use next/image, and adopt Next's ESLint rules (Core Web Vitals)

**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks rules)
Follow existing code style and naming conventions; maintain clean code standards (measured by SonarQube)

Files:

  • components/voting/VotingModal.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Must pass tsc --noEmit type checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") over React. namespace usage (React.useMemo, React.useRef, etc.)
If the react-hooks/exhaustive-deps lint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic in useEffectEvent

**/*.{ts,tsx}: Must pass tsc --noEmit for TypeScript type checking
Prefer Server Components over Client Components; use Server Functions/Server Actions ('use server') for mutations
Remove unnecessary Effects; if Effect only derives state, compute during render instead
Use useEffectEvent for non-reactive logic inside Effects to avoid unnecessary re-runs
Use framework APIs: <Link> for internal links, next/image for images, adopt Next's ESLint rules
Use 'use cache' directive and Cache Components features for explicit opt-in caching in Next.js 16
Use TypeScript and React functional components with hooks
When parsing Seize URLs or similar, fail fast if base origin is unavailable; do not fall back to placeholder origins
Replace <img> elements with <Image /> from next/image
Use <Link href="/path"> for internal navigation instead of plain HTML links
Move data fetches to Server Components; handle mutations through Server Functions/Server Actions with 'use server' directive

Files:

  • components/voting/VotingModal.tsx
🧠 Learnings (6)
📚 Learning: 2025-12-03T14:52:34.255Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.255Z
Learning: Fix with modernization (no `// eslint-disable` unless explicitly instructed); prefer refactors aligned with React 19.2, React Compiler, and Next.js 16 conventions

Applied to files:

  • components/voting/VotingModal.tsx
📚 Learning: 2025-12-05T10:55:43.458Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.458Z
Learning: Applies to __tests__/**/{components,contexts,hooks}/**/*.{ts,tsx} : Use semantic HTML elements (`<label>`, `<output>`) over ARIA attributes when possible; every form control must have a label

Applied to files:

  • components/voting/VotingModal.tsx
📚 Learning: 2025-12-05T10:55:30.859Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-05T10:55:30.859Z
Learning: Applies to **/*.{ts,tsx} : Prefer Server Components over Client Components; use Server Functions/Server Actions (`'use server'`) for mutations

Applied to files:

  • components/voting/VotingModal.tsx
📚 Learning: 2025-12-05T10:55:30.859Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-05T10:55:30.859Z
Learning: Applies to **/*.{ts,tsx} : Move data fetches to Server Components; handle mutations through Server Functions/Server Actions with `'use server'` directive

Applied to files:

  • components/voting/VotingModal.tsx
📚 Learning: 2025-12-05T10:55:30.859Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-05T10:55:30.859Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript and React functional components with hooks

Applied to files:

  • components/voting/VotingModal.tsx
📚 Learning: 2025-12-03T14:52:34.255Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.255Z
Learning: Prefer Server Components over Client Components where possible; data reads should use Server Components with inline fetches; mutations should use Server Functions / Server Actions (`'use server'`); avoid client Effects for data fetching unless truly needed

Applied to files:

  • components/voting/VotingModal.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
components/voting/VotingModal.tsx (1)

52-85: Portal-based modal structure and ARIA wiring look correct

The portalized modalContent looks good: the dialog element is focusable, uses role="dialog", aria-modal="true", and aria-labelledby={titleId}, and the same titleId is passed through to ModalLayout for proper labelling. Backdrop vs. content click handling is correctly separated so backdrop clicks close the modal while inner content clicks do not. No issues from a structural or accessibility standpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants