fix: show auth config notice on Sign In button when Clerk key is missing#486
fix: show auth config notice on Sign In button when Clerk key is missing#486Xenon010101 wants to merge 1 commit into
Conversation
✅ Deploy Preview for astounding-nougat-da0f6a ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@Xenon010101 is attempting to deploy a commit to the adityapaul2603-gmailcom's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR improves the UX when ChangesAuth Configuration Fallback UI
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Quick context for review: This directly addresses issue 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Navbar.jsx`:
- Around line 326-336: Currently the "Sign In" disabled button and its tooltip
(the element containing VITE_CLERK_PUBLISHABLE_KEY) rely on group-hover and are
not discoverable via keyboard or touch; update the Navbar.jsx sign-in block so
the helper message is always visible (or at least visible when the wrapper
receives focus) instead of using group-hover opacity, e.g. render a persistent
helper text element beneath the disabled Sign In button (the button with class
"theme-button-primary" and the surrounding "relative group" wrapper) or make the
wrapper focusable and wire aria-describedby on the button to the tooltip element
(the absolute div that mentions VITE_CLERK_PUBLISHABLE_KEY) so keyboard and
screen-reader users can discover the notice without hover.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 425876aa-b061-4078-9c33-3d72fec2d435
📒 Files selected for processing (1)
src/components/Navbar.jsx
| <div className="relative group"> | ||
| <button | ||
| title="Auth not configured" | ||
| disabled | ||
| className="theme-button-primary relative group overflow-hidden rounded-xl border border-slate-200 dark:border-slate-700 bg-slate-50 dark:bg-slate-900/40 text-slate-700 dark:text-slate-200 px-6 py-2 text-sm font-bold transition-all duration-300 shadow-md opacity-50 cursor-not-allowed" | ||
| className="theme-button-primary rounded-xl border border-slate-200 dark:border-slate-700 bg-slate-50 dark:bg-slate-900/40 text-slate-500 dark:text-slate-500 px-6 py-2 text-sm font-bold opacity-50 cursor-not-allowed" | ||
| > | ||
| Sign In | ||
| </button> | ||
| </> | ||
| <div className="absolute right-0 top-full mt-2 w-64 rounded-lg border border-amber-200 dark:border-amber-800 bg-amber-50 dark:bg-amber-950 px-3 py-2 text-xs text-amber-700 dark:text-amber-300 shadow-lg opacity-0 group-hover:opacity-100 transition-opacity duration-200 pointer-events-none z-50"> | ||
| Authentication not configured. Set <code className="font-mono text-amber-800 dark:text-amber-200">VITE_CLERK_PUBLISHABLE_KEY</code> in your <code className="font-mono text-amber-800 dark:text-amber-200">.env</code> file. | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Make the missing-auth notice non-hover-only (keyboard + touch accessible).
Right now the warning depends on group-hover, but both fallback buttons are disabled (not focusable), and mobile users typically can’t hover. This means the config message can remain undiscoverable in the exact failure mode this PR targets.
A simple fix is to render a persistent helper text under the disabled button (or show it via focusable wrapper + aria-describedby), instead of hover-only tooltip behavior.
As per coding guidelines, src/components/** should “focus on React best practices … Ensure styles follow Tailwind CSS patterns.” A critical notice should not rely on hover-only Tailwind interaction for discoverability.
Also applies to: 459-469
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Navbar.jsx` around lines 326 - 336, Currently the "Sign In"
disabled button and its tooltip (the element containing
VITE_CLERK_PUBLISHABLE_KEY) rely on group-hover and are not discoverable via
keyboard or touch; update the Navbar.jsx sign-in block so the helper message is
always visible (or at least visible when the wrapper receives focus) instead of
using group-hover opacity, e.g. render a persistent helper text element beneath
the disabled Sign In button (the button with class "theme-button-primary" and
the surrounding "relative group" wrapper) or make the wrapper focusable and wire
aria-describedby on the button to the tooltip element (the absolute div that
mentions VITE_CLERK_PUBLISHABLE_KEY) so keyboard and screen-reader users can
discover the notice without hover.
Description
When VITE_CLERK_PUBLISHABLE_KEY is not configured, the Sign In button now shows a hover-visible warning tooltip with a clear message.
Related Issue
Fixes #445
Summary by CodeRabbit