Clicking logo on /pricing to homepage-fixed#350
Clicking logo on /pricing to homepage-fixed#350PrabhakaranVijayan wants to merge 1 commit intoapsinghdev:mainfrom
Conversation
|
@PrabhakaranVijayan is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe navbar branding block (logo and "Opensox AI" text) is converted from a non-clickable div to a clickable Link component directing to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/landing-sections/navbar.tsx`:
- Line 86: The brand image in the Navbar component uses non-descriptive
alt="background"; update the <img> in the Navbar (component/function name:
Navbar) to use meaningful alt text (e.g., "YourBrand logo" or a descriptive
label) or accept an alt prop and fall back to a descriptive string so screen
readers get useful context; locate the <img> with alt="background" and replace
it with a descriptive alt value or prop usage.
- Line 82: The logo Link currently routes to "/dashboard/home" — update the href
on the JSX Link element (the <Link ... className="text-xl md:text-2xl
font-medium tracking-tighter flex items-center gap-2"> used for the logo) so
clicking the logo from pages like /pricing navigates to the landing homepage by
changing the target to "/".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33fa87e3-5494-48fb-a2d3-cfa6f9f1cbe7
📒 Files selected for processing (1)
apps/web/src/components/landing-sections/navbar.tsx
| </div> | ||
| <span>Opensox AI</span> | ||
| <div > | ||
| <Link href="/dashboard/home" className="text-xl md:text-2xl font-medium tracking-tighter flex items-center gap-2"> |
There was a problem hiding this comment.
logo link target does not match the stated fix
Line 82 routes to /dashboard/home, but the PR objective says clicking the logo on /pricing should go to homepage. If homepage is the landing root, this should be /.
proposed fix
- <Link href="/dashboard/home" className="text-xl md:text-2xl font-medium tracking-tighter flex items-center gap-2">
+ <Link href="/" className="text-xl md:text-2xl font-medium tracking-tighter flex items-center gap-2">📝 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.
| <Link href="/dashboard/home" className="text-xl md:text-2xl font-medium tracking-tighter flex items-center gap-2"> | |
| <Link href="/" className="text-xl md:text-2xl font-medium tracking-tighter flex items-center gap-2"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/landing-sections/navbar.tsx` at line 82, The logo
Link currently routes to "/dashboard/home" — update the href on the JSX Link
element (the <Link ... className="text-xl md:text-2xl font-medium
tracking-tighter flex items-center gap-2"> used for the logo) so clicking the
logo from pages like /pricing navigates to the landing homepage by changing the
target to "/".
| <div className="w-8 md:w-10 aspect-square overflow-hidden relative"> | ||
| <Image | ||
| src="/assets/logo.svg" | ||
| alt="background" |
There was a problem hiding this comment.
use meaningful alt text for the brand image
Line 86 uses alt="background", which is not descriptive for assistive tech.
proposed fix
- alt="background"
+ alt="Opensox logo"As per coding guidelines, "Provide alt text for images".
📝 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.
| alt="background" | |
| alt="Opensox logo" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/landing-sections/navbar.tsx` at line 86, The brand
image in the Navbar component uses non-descriptive alt="background"; update the
<img> in the Navbar (component/function name: Navbar) to use meaningful alt text
(e.g., "YourBrand logo" or a descriptive label) or accept an alt prop and fall
back to a descriptive string so screen readers get useful context; locate the
<img> with alt="background" and replace it with a descriptive alt value or prop
usage.
1 participant
issue #348 is fixed
Summary by CodeRabbit