-
-
Notifications
You must be signed in to change notification settings - Fork 481
update outbound link ui #542
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis set of changes focuses on codebase cleanup, UI simplification, and component restructuring. Key updates include refactoring several components for more compact and readable JSX, consolidating and flattening import statements, and removing redundant or obsolete components. The "Outbound Links" functionality is now integrated as a tab within the main Events section, with corresponding updates to data fetching and UI rendering logic. No significant changes to logic or behavior are introduced outside of these structural and stylistic improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EventsComponent
participant Events_
participant OutboundLinks
User->>EventsComponent: Selects a tab ("Custom Events" or "Outbound Links")
alt "Custom Events" tab
EventsComponent->>Events_: Render Events_ component
Events_->>Events_: Fetch event names
Events_->>Events_: Render loader, header, and event list
else "Outbound Links" tab
EventsComponent->>OutboundLinks: Render OutboundLinks component
OutboundLinks->>OutboundLinks: Fetch outbound link data
OutboundLinks->>OutboundLinks: Render loader, header, and outbound links list
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 3
🔭 Outside diff range comments (1)
client/src/app/[site]/components/shared/StandardSection/StandardSectionDialog.tsx (1)
93-99: Addrel="noopener noreferrer"on external linkOpening an URL in a new tab without
rel="noopener noreferrer"allows the target page to accesswindow.opener, which can trigger reverse-tab-nabbing attacks.- <a href={getLink(row.original)} target="_blank" onClick={(e) => e.stopPropagation()}> + <a + href={getLink(row.original)} + target="_blank" + rel="noopener noreferrer" + onClick={(e) => e.stopPropagation()} + >
🧹 Nitpick comments (3)
client/src/app/[site]/components/shared/StandardSection/StandardSectionDialog.tsx (1)
1-12: Re-order imports to match project guidelinesInternal alias paths (
@/…) precede external packages (@tanstack/*,lucide-react, etc.).
Per the coding-guidelines, imports should be grouped “external → internal” and alphabetised within each group.
Re-ordering avoids churn in future diffs and keeps style consistent.client/src/app/[site]/main/components/sections/Events.tsx (1)
11-30: RenameEvents_and defer data-fetch until tab is active
Events_(underscore suffix) reads like a generated placeholder and violates our camel/Pascal naming rules.
ConsiderCustomEventsTab(or similar) and move the hook call inside the<TabsContent>render so the request fires only when the tab is visible.
This avoids double network traffic when users never open the tab.client/src/app/[site]/events/components/OutboundLinksList.tsx (1)
4-4: Remove unusedExternalLinkimport
ExternalLinkis no longer referenced and will trigger ts/ eslint warnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
client/src/app/[site]/components/shared/StandardSection/Row.tsx(3 hunks)client/src/app/[site]/components/shared/StandardSection/StandardSectionDialog.tsx(11 hunks)client/src/app/[site]/events/components/OutboundLinksList.tsx(4 hunks)client/src/app/[site]/main/components/sections/Events.tsx(1 hunks)client/src/app/[site]/main/components/sections/OutboundLinks.tsx(0 hunks)client/src/app/[site]/main/page.tsx(0 hunks)client/src/app/[site]/performance/components/PerformanceTable.tsx(15 hunks)
💤 Files with no reviewable changes (2)
- client/src/app/[site]/main/page.tsx
- client/src/app/[site]/main/components/sections/OutboundLinks.tsx
🧰 Additional context used
📓 Path-based instructions (2)
{client,server}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
{client,server}/**/*.{ts,tsx}: Use TypeScript with strict typing throughout both client and server
Use try/catch blocks with specific error types for error handling
Use camelCase for variables and functions, PascalCase for components and types
Group imports by external, then internal, and sort alphabetically within groups
Files:
client/src/app/[site]/performance/components/PerformanceTable.tsxclient/src/app/[site]/components/shared/StandardSection/Row.tsxclient/src/app/[site]/components/shared/StandardSection/StandardSectionDialog.tsxclient/src/app/[site]/events/components/OutboundLinksList.tsxclient/src/app/[site]/main/components/sections/Events.tsx
client/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Frontend: Use Next.js, Tailwind CSS, Shadcn UI, Tanstack Query, Zustand, Luxon, Nivo, and react-hook-form
Files:
client/src/app/[site]/performance/components/PerformanceTable.tsxclient/src/app/[site]/components/shared/StandardSection/Row.tsxclient/src/app/[site]/components/shared/StandardSection/StandardSectionDialog.tsxclient/src/app/[site]/events/components/OutboundLinksList.tsxclient/src/app/[site]/main/components/sections/Events.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.559Z
Learning: Applies to client/**/*.{tsx} : Client: Use React functional components with minimal useEffect and inline functions
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.559Z
Learning: Applies to client/**/* : Frontend: Use Next.js, Tailwind CSS, Shadcn UI, Tanstack Query, Zustand, Luxon, Nivo, and react-hook-form
📚 Learning: 2025-08-03T17:30:25.559Z
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.559Z
Learning: Applies to client/**/*.{tsx} : Client: Use React functional components with minimal useEffect and inline functions
Applied to files:
client/src/app/[site]/performance/components/PerformanceTable.tsxclient/src/app/[site]/components/shared/StandardSection/Row.tsxclient/src/app/[site]/components/shared/StandardSection/StandardSectionDialog.tsxclient/src/app/[site]/events/components/OutboundLinksList.tsxclient/src/app/[site]/main/components/sections/Events.tsx
📚 Learning: 2025-08-03T17:30:25.559Z
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.559Z
Learning: Applies to {client,server}/**/*.{ts,tsx} : Group imports by external, then internal, and sort alphabetically within groups
Applied to files:
client/src/app/[site]/performance/components/PerformanceTable.tsxclient/src/app/[site]/components/shared/StandardSection/StandardSectionDialog.tsx
📚 Learning: 2025-08-03T17:30:25.559Z
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.559Z
Learning: Applies to client/**/* : Frontend: Use Next.js, Tailwind CSS, Shadcn UI, Tanstack Query, Zustand, Luxon, Nivo, and react-hook-form
Applied to files:
client/src/app/[site]/performance/components/PerformanceTable.tsxclient/src/app/[site]/components/shared/StandardSection/StandardSectionDialog.tsxclient/src/app/[site]/main/components/sections/Events.tsx
| <a href={getLink(item)} target="_blank" onClick={(e) => e.stopPropagation()}> | ||
| <SquareArrowOutUpRight | ||
| className="w-3 h-3 text-neutral-300 hover:text-neutral-100" | ||
| className="ml-0.5 w-3.5 h-3.5 text-neutral-300 hover:text-neutral-100" | ||
| strokeWidth={3} | ||
| /> | ||
| </a> |
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.
🛠️ Refactor suggestion
Harden external link – missing rel attribute
Same security concern as in StandardSectionDialog.
Add:
- <a href={getLink(item)} target="_blank" onClick={(e) => e.stopPropagation()}>
+ <a
+ href={getLink(item)}
+ target="_blank"
+ rel="noopener noreferrer"
+ onClick={(e) => e.stopPropagation()}
+ >📝 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.
| <a href={getLink(item)} target="_blank" onClick={(e) => e.stopPropagation()}> | |
| <SquareArrowOutUpRight | |
| className="w-3 h-3 text-neutral-300 hover:text-neutral-100" | |
| className="ml-0.5 w-3.5 h-3.5 text-neutral-300 hover:text-neutral-100" | |
| strokeWidth={3} | |
| /> | |
| </a> | |
| <a | |
| href={getLink(item)} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| <SquareArrowOutUpRight | |
| className="ml-0.5 w-3.5 h-3.5 text-neutral-300 hover:text-neutral-100" | |
| strokeWidth={3} | |
| /> | |
| </a> |
🤖 Prompt for AI Agents
In client/src/app/[site]/components/shared/StandardSection/Row.tsx around lines
70 to 75, the external link anchor tag is missing the rel attribute, which is a
security risk. Add rel="noopener noreferrer" to the <a> tag to prevent security
vulnerabilities when using target="_blank".
| "z-5 mx-2 flex justify-between items-center w-full", | ||
| size === "small" ? "text-xs" : "text-sm" | ||
| )} |
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.
💡 Verification agent
🧩 Analysis chain
Non-standard Tailwind class z-5
Tailwind’s default z-index scale is 0,10,20…50.
If a custom value isn’t configured, the class will be stripped, breaking stacking order.
Use z-10 or a custom arbitrary value: z-[5].
🏁 Script executed:
#!/bin/bash
# Locate Tailwind config file(s)
fd tailwind.config
# Search for custom zIndex extensions
rg -n "zIndex" -A5 tailwind.config.js tailwind.config.ts || trueLength of output: 228
🏁 Script executed:
#!/bin/bash
# Search for zIndex extensions in the client Tailwind config
rg -n "zIndex" -A5 client/tailwind.config.tsLength of output: 44
Non-standard Tailwind class z-5 detected
Tailwind CSS does not include a z-5 utility by default (its built-in scale is 0, 10, 20…50). Since there’s no theme.extend.zIndex entry for 5 in client/tailwind.config.ts, your z-5 class will be purged at build time, breaking the intended stacking order.
Please replace it with one of the following:
- A default value:
- "z-5 mx-2 flex justify-between items-center w-full" + "z-10 mx-2 flex justify-between items-center w-full"
- An arbitrary value:
- "z-5 mx-2 flex justify-between items-center w-full" + "z-[5] mx-2 flex justify-between items-center w-full"
File: client/src/app/[site]/events/components/OutboundLinksList.tsx
Lines: 40–42
📝 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.
| "z-5 mx-2 flex justify-between items-center w-full", | |
| size === "small" ? "text-xs" : "text-sm" | |
| )} | |
| "z-10 mx-2 flex justify-between items-center w-full", | |
| size === "small" ? "text-xs" : "text-sm" | |
| )} |
🤖 Prompt for AI Agents
In client/src/app/[site]/events/components/OutboundLinksList.tsx around lines 40
to 42, the Tailwind class "z-5" is non-standard and will be purged since it is
not defined in the Tailwind config. Replace "z-5" with a valid default z-index
class like "z-10" or use an arbitrary value such as "z-[5]" to maintain the
intended stacking order.
| <SquareArrowOutUpRight | ||
| className="w-3 h-3 text-neutral-300 hover:text-neutral-100" | ||
| className="ml-0.5 w-3.5 h-3.5 text-neutral-300 hover:text-neutral-100" | ||
| strokeWidth={3} | ||
| /> | ||
| </a> |
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.
Add rel attribute and guard against undefined domain
- Security – add
rel="noopener noreferrer"(see previous comments). - If
siteMetadata?.domainis falsy the generated URL becomeshttps://undefined…, producing broken links.
- href={`https://${siteMetadata?.domain}${value}`}
+ href={
+ siteMetadata?.domain
+ ? `https://${siteMetadata.domain}${value}`
+ : undefined
+ }
target="_blank"
+ rel="noopener noreferrer"
onClick={(e) => e.stopPropagation()}When domain is unavailable, omit the anchor entirely or fall back to plain text.
📝 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.
| <SquareArrowOutUpRight | |
| className="w-3 h-3 text-neutral-300 hover:text-neutral-100" | |
| className="ml-0.5 w-3.5 h-3.5 text-neutral-300 hover:text-neutral-100" | |
| strokeWidth={3} | |
| /> | |
| </a> | |
| <a | |
| href={ | |
| siteMetadata?.domain | |
| ? `https://${siteMetadata.domain}${value}` | |
| : undefined | |
| } | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| <SquareArrowOutUpRight | |
| className="ml-0.5 w-3.5 h-3.5 text-neutral-300 hover:text-neutral-100" | |
| strokeWidth={3} | |
| /> | |
| </a> |
🤖 Prompt for AI Agents
In client/src/app/[site]/performance/components/PerformanceTable.tsx around
lines 186 to 190, the anchor tag lacks the rel="noopener noreferrer" attribute
for security and uses siteMetadata?.domain without checking if it is defined,
causing broken links if domain is falsy. Fix this by adding rel="noopener
noreferrer" to the anchor tag and conditionally rendering the anchor only if
siteMetadata?.domain is truthy; otherwise, render plain text or omit the link to
avoid broken URLs.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Refactor