Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a search fix and component refactoring by adding conditional keyboard shortcut functionality to the search component and consolidating split layout components into reusable layout components.
- Adds conditional keyboard shortcut support to SearchBar component
- Refactors multiple hero and media-split components to use new SplitLayoutLeft/SplitLayoutRight components
- Creates new split-layout component with configurable padding and max-width variants
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| registry/new-york/ui/search-bar.tsx | Adds guard to prevent keyboard shortcut registration when shortcutKey is falsy |
| registry/new-york/section/pricing-10.tsx | Removes align="center" prop from Container component |
| registry/new-york/section/media-split-04.tsx | Refactors to use SplitLayoutRight component instead of custom section layout |
| registry/new-york/section/media-split-03.tsx | Refactors to use SplitLayoutLeft component instead of custom section layout |
| registry/new-york/section/hero-19.tsx | Refactors to use SplitLayoutLeft component instead of custom section layout |
| registry/new-york/section/hero-17.tsx | Refactors to use SplitLayoutRight component instead of custom section layout |
| registry/new-york/layout/split-layout.tsx | Creates new reusable split layout components with configurable variants |
| components/app-layout/library-search.tsx | Adds shortcutKey prop with default value and passes it to SearchBar |
| components/app-layout/app-header.tsx | Configures different shortcut keys for desktop vs mobile search instances |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <img | ||
| src={imageUrl} | ||
| alt={imageAlt} | ||
| className="block lg:hidden w-full h-full object-cover" | ||
| /> |
There was a problem hiding this comment.
The mobile image appears after the desktop image container but outside of it. This creates duplicate image elements and could lead to layout issues. Consider consolidating the image rendering logic or adding proper responsive classes to a single image element.
| <img | ||
| src={imageUrl} | ||
| alt={imageAlt} | ||
| className="block lg:hidden w-full h-full object-cover" | ||
| /> |
There was a problem hiding this comment.
Same issue as SplitLayoutLeft - duplicate image elements for mobile and desktop views. This pattern duplicates DOM elements and could cause accessibility issues with screen readers encountering the same image twice.
No description provided.