-
Notifications
You must be signed in to change notification settings - Fork 646
portal: landing page ui improvements #8647
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
base: main
Are you sure you want to change the base?
Conversation
|
@MananTank is attempting to deploy a commit to the thirdweb Team on Vercel. A member of the Team first needs to authorize it. |
|
WalkthroughCentralized and simplified the main page: replaced the two-column hero with a centered hero and reorganized content into card-based sections (Card, MinimalCard, SectionHeader, ProductsSection, ReferenceSection, ArchiveSection). Also added an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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: 2
🤖 Fix all issues with AI agents
In `@apps/portal/src/app/page.tsx`:
- Around line 35-67: In the Hero component, the external Link used inside the
Button (the Link with href="https://playground.thirdweb.com" and
target="_blank") should include rel="noopener noreferrer" to prevent
reverse-tabnabbing; update the Link element in Hero (the Link nested in
Button/asChild) to pass rel="noopener noreferrer" to the underlying anchor.
- Around line 222-274: The external links opened with target="_blank" in the
Card (the first Link block rendering props.icon/props.title/props.description)
and in MinimalCard are missing rel="noopener noreferrer"; update both Link
elements to include rel={props.external ? "noopener noreferrer" : undefined}
(use the existing props.external boolean) so the Link rendering in the Card and
the MinimalCard components sets the rel attribute only for external links.
🧹 Nitpick comments (3)
apps/portal/src/components/AI/chat-button.tsx (1)
3-23: Add explicit return types and a props alias.Inline prop typing and missing return types make the component less consistent with the TS guidelines; also consider extracting
ChatLoadingto its own file to keep one stateless function per file.♻️ Proposed refactor
-import { lazy, Suspense, useCallback, useState } from "react"; +import type React from "react"; +import { lazy, Suspense, useCallback, useState } from "react"; -export function ChatButton(props: { className?: string }) { +type ChatButtonProps = { className?: string }; + +export function ChatButton(props: ChatButtonProps): React.JSX.Element {Apply the same explicit return type pattern to
ChatLoading. As per coding guidelines, please add explicit return types and keep one stateless function per file.apps/portal/src/app/page.tsx (2)
71-80: Add explicit return types and consider splitting components into separate files.All components in this file lack explicit return types, and multiple stateless components live in a single file. Consider adding return types and extracting components into separate files to align with the single-function-per-file rule.
♻️ Example adjustment
+type SectionHeaderProps = { title: string; description: string }; -function SectionHeader(props: { title: string; description: string }) { +function SectionHeader(props: SectionHeaderProps): React.JSX.Element {(You may need a type-only React import if you use
React.JSX.Element.) As per coding guidelines, please add explicit return types and keep one stateless function per file.
214-275: Deduplicate card prop types into a shared alias.
CardandMinimalCardshare the same prop shape; extracting a shared type (ideally in a localtypes.tsor@/types) reduces duplication.♻️ Example refactor
+type BaseCardProps = { + title: string; + description?: string; + href: string; + icon: React.FC<{ className?: string }>; + external?: boolean; +}; + -function Card(props: { - title: string; - description?: string; - href: string; - icon: React.FC<{ className?: string }>; - external?: boolean; -}) { +function Card(props: BaseCardProps) { ... } -function MinimalCard(props: { - title: string; - description?: string; - href: string; - icon: React.FC<{ className?: string }>; - external?: boolean; -}) { +function MinimalCard(props: BaseCardProps) { ... }As per coding guidelines, please re-use shared types via a local
types.tsor@/types.
PR-Codex overview
This PR focuses on enhancing the
ChatButtoncomponent and restructuring thePagecomponent by updating its layout and improving the presentation of sections related to documentation and SDKs.Detailed summary
ChatButtonto accept aclassNameprop for styling.LearningResourcesSectionwithProductsSectionand modified its content.SectionHeaderfor consistent section titles.CardandMinimalCardcomponents for better presentation.Herosection, including new CTAs.ReferenceSectionandArchiveSectionwith new cards.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.