-
-
Notifications
You must be signed in to change notification settings - Fork 398
Fix ModeToggle Tooltip Misalignment #2873
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
Conversation
WalkthroughUpdated tooltip positioning in the ModeToggle component by adding an explicit Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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.
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)
frontend/src/components/ModuleForm.tsx (1)
278-292: Better surfacing of suggestion errors; consider whether to expose raw error text.Using
setErrorin the catch block gives users clear feedback when fetching suggestions fails and is consistent with the existingerrorstate handling in this component, so the behavioral change looks good.One thing to consider:
err.message(from Apollo/GraphQL) can contain low-level or internal details that you might not want to show directly to end users. You may want to keep the console/log message detailed while presenting a friendlier, more generic message in the UI, depending on your error-handling policy.Based on learnings, this continues to rely on native HTML
requiredvalidation for the form as expected and only adds contextual feedback for the async suggestions flow.frontend/src/components/ModeToggle.tsx (1)
24-28: Tooltip placement change aligns with misalignment fix scope.Adding
placement="bottom-end"(withshowArrow) should help the tooltip align more precisely with the mode toggle button while keeping content and accessibility unchanged. This looks appropriate for resolving the positioning part of #2711.If you intend this PR to fully close that issue, note that the tooltip color/contrast behavior described there still appears to rely on the default Tooltip styling and may need a follow-up tweak.
Please double‑check in both logged‑in and logged‑out states that the tooltip points to the ModeToggle itself (and not the Sign In/Profile controls) across common viewport widths.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/app/api/auth/[...nextauth]/route.ts(2 hunks)frontend/src/components/ModeToggle.tsx(1 hunks)frontend/src/components/ModuleForm.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-10T11:08:47.258Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/api/auth/[...nextauth]/route.ts:13-25
Timestamp: 2025-08-10T11:08:47.258Z
Learning: In the OWASP Nest codebase (frontend/src/app/api/auth/[...nextauth]/route.ts), input validation and string trimming for authentication-related queries like `isProjectLeader` and `isMentor` are handled in the backend rather than the frontend. The backend is responsible for sanitizing and validating input parameters.
Applied to files:
frontend/src/app/api/auth/[...nextauth]/route.ts
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/components/ModuleForm.tsx
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/ModeToggle.tsx
🔇 Additional comments (2)
frontend/src/app/api/auth/[...nextauth]/route.ts (2)
20-23: More informative project‑leader error propagation looks good.Including the original
err.messagein the thrownErrormakes failures incheckIfProjectLeadereasier to debug while preserving the existing behavior of failing the JWT callback on query errors.
36-39: Mentor‑status error messaging is now consistent with leader check.Same improvement here: the new error text includes the original cause, which should help when diagnosing mentor‑status lookup issues, with no change to control flow.
b956103 to
0e8db60
Compare
0e8db60 to
8fd1cce
Compare
8fd1cce to
46104a3
Compare
|
kasya
left a comment
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.
@nileshpahari thanks for updating this! 👍🏼



Proposed change
Fixes the misalignment of the tooltip, which is currently pointing to the SignIn button (when not logged in) and Profile Icon (when logged in) rather than the ModeToggle button.
Resolves #2711
Files changed:
frontend/src/components/ModeToggle.tsxFix:
Adding
placement="bottom-end"to theTooltipsolves the issue and improves clarity.Checklist
make check-testlocally and all tests passed