Conversation
Breaking Change: The `theme` prop is now required for all logos components. - Remove default value "auto" from theme prop in all section components (logos-00 to logos-18) - Remove default value from theme prop in all marketing logos components - Update all 81 examples to explicitly include theme="auto" - Ensures developers must explicitly choose theme behavior to prevent visual bugs This change prevents accidental misconfiguration where logos appear incorrectly due to missing theme settings. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
… split/stack variants Breaking Change: Split-media components have been reorganized into a clearer naming structure. Component renames: - split-media-00 → media-split-01 - split-media-01 → media-split-02 - split-media-02 → media-split-03 - split-media-03 → media-split-04 - split-media-04 → media-split-05 - split-media-05 → media-split-06 - split-media-06 → media-stack-01 (vertical layout) - split-media-07 → media-split-07 - split-media-08 → media-split-08 Changes: - Renamed SplitMedia to MediaSplit for horizontal layouts - Renamed SplitMedia to MediaStack for vertical layouts - Updated all interfaces from SplitMediaProps to MediaSplitProps/MediaStackProps - Moved examples to new directories: media-split/ and media-stack/ - Updated section title from "Split Media" to "Media" - Clear separation between horizontal (media-split-*) and vertical (media-stack-*) layouts This restructuring provides clearer naming that immediately indicates the layout type, making the component library more intuitive and easier to navigate. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors component naming and API design to improve consistency and usability. The main changes are renaming "Split Media" components to "Media" category with better naming conventions, requiring the theme prop on logo components, and adding navigation improvements.
- Rename
SplitMediacomponents toMediaSplitandMediaStackfor clarity - Make
themeprop required on all logo components for better API consistency - Update navbar with background styling for better visual hierarchy
Reviewed Changes
Copilot reviewed 129 out of 129 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| registry/new-york/section/media-*.tsx | Renamed components from SplitMedia to MediaSplit/MediaStack with updated interfaces |
| registry/new-york/section/logos-*.tsx | Changed theme prop from optional to required with removal of default values |
| registry/new-york/marketing/logos-*.tsx | Updated all logo marketing components to require theme prop |
| registry/new-york/navigation/navbar-desktop.tsx | Added background and border styling for better visual contrast |
| examples/ | Updated all example files to provide explicit theme prop values |
| registry-source.json | Updated component paths and titles to reflect new naming scheme |
| data/section.ts | Reorganized navigation structure with new "Media" category and updated component references |
Comments suppressed due to low confidence (1)
examples/section/logos/Logos15ColorExample.tsx:1
- Import path is incorrect. Should be
@/registry/new-york/section/logos-15to match the other examples.
import { Logos } from "@/components/section/logos-15";
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 134 out of 134 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <motion.img | ||
| {...withDelay(fadeInUp, 0.1)} | ||
| src={imageSrc} | ||
| alt={imageAlt || ""} | ||
| className={cn("w-full rounded-lg object-cover", imageClassName)} | ||
| /> |
There was a problem hiding this comment.
The image element has been changed from being wrapped in a motion.div to a motion.img directly. This could affect animation behavior and styling inheritance. Consider maintaining consistent animation wrapper patterns across similar components.
| <motion.img | |
| {...withDelay(fadeInUp, 0.1)} | |
| src={imageSrc} | |
| alt={imageAlt || ""} | |
| className={cn("w-full rounded-lg object-cover", imageClassName)} | |
| /> | |
| <motion.div {...withDelay(fadeInUp, 0.1)}> | |
| <img | |
| src={imageSrc} | |
| alt={imageAlt || ""} | |
| className={cn("w-full rounded-lg object-cover", imageClassName)} | |
| /> | |
| </motion.div> |
| <motion.div {...withDelay(fadeInUp, 0.1)}> | ||
| <img | ||
| src={imageSrc} | ||
| alt={imageAlt || ""} | ||
| className={cn("w-full rounded-lg object-cover", imageClassName)} | ||
| /> | ||
| </motion.div> |
There was a problem hiding this comment.
This component maintains the motion.div wrapper around the img element, while media-split-08.tsx uses motion.img directly. This inconsistency in animation wrapper patterns could lead to different behavior and maintenance issues.
| <motion.div {...withDelay(fadeInUp, 0.1)}> | |
| <img | |
| src={imageSrc} | |
| alt={imageAlt || ""} | |
| className={cn("w-full rounded-lg object-cover", imageClassName)} | |
| /> | |
| </motion.div> | |
| <motion.img | |
| {...withDelay(fadeInUp, 0.1)} | |
| src={imageSrc} | |
| alt={imageAlt || ""} | |
| className={cn("w-full rounded-lg object-cover", imageClassName)} | |
| /> |
No description provided.