add support for search bar and subtle UI changes#1
add support for search bar and subtle UI changes#10xkafkaa wants to merge 1 commit intopavangudiwada:mainfrom
Conversation
|
@0xkafkaa is attempting to deploy a commit to the pavangudiwada's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @0xkafkaa, thanks for the PR! I'll review it soon |
|
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 the job board listing page to support a global search bar that searches across all job boards, along with subtle UI improvements to the layout and navigation. The main architectural change is extracting the page layout into a client component to enable interactive search functionality.
- Adds a new reusable SearchBar component with search icon
- Refactors the category page to use a new ClientJobBoardLayout component that supports search state management
- Enhances the sidebar navigation with improved styling and category descriptions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| app/components/SearchBar.tsx | New search bar component with search icon and text input field |
| app/[category]/page.tsx | Refactored to extract UI logic into ClientJobBoardLayout, now passes data as props |
| app/[category]/ClientJobBoardLayout.tsx | New client component handling search state, filtering logic, and responsive layout with enhanced navigation styling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </div> | ||
|
|
||
| {filteredBoards.length > 0 ? ( | ||
| <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 xl:grid-cols-3 gap-6"> |
There was a problem hiding this comment.
The grid layout specifies both 'lg:grid-cols-3' and 'xl:grid-cols-3', which is redundant since they specify the same value. Consider removing 'xl:grid-cols-3' or changing it to a different value if a different layout is intended for extra-large screens.
| <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 xl:grid-cols-3 gap-6"> | |
| <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-6"> |
| </p> | ||
| <button | ||
| onClick={() => setSearchQuery("")} | ||
| className="text-blue-600 hover:text-blue-800 font-medium hover:underline" |
There was a problem hiding this comment.
The button in the empty state is missing an aria-label or other accessible name to describe its action. While the text content provides some context, adding an aria-label like "Clear search query" would improve accessibility for screen reader users.
| className="text-blue-600 hover:text-blue-800 font-medium hover:underline" | |
| className="text-blue-600 hover:text-blue-800 font-medium hover:underline" | |
| aria-label="Clear search query and browse all job boards" |
| <input | ||
| type="text" | ||
| className="block w-full text-gray-900 pl-10 pr-3 py-2 border border-gray-300 rounded-md leading-5 bg-white placeholder-gray-500 focus:outline-none focus:placeholder-gray-400 focus:ring-1 focus:ring-slate-900 focus:border-slate-900 sm:text-sm transition duration-150 ease-in-out" | ||
| placeholder={placeholder} |
There was a problem hiding this comment.
The search input field is missing an aria-label or associated label element. This makes it difficult for screen reader users to understand the purpose of the input field. Consider adding an aria-label attribute to the input element to describe its purpose.
| placeholder={placeholder} | |
| placeholder={placeholder} | |
| aria-label={placeholder || "Search"} |
| <Link | ||
| key={cat.id} | ||
| href={`/${cat.id}`} | ||
| onClick={() => setSearchQuery("")} // Clear search when changing category | ||
| className={`block py-2.5 px-4 rounded-lg transition-colors ${cat.id === currentCategory | ||
| ? 'bg-blue-600 text-white font-medium shadow-sm' | ||
| : 'text-gray-300 hover:bg-slate-800 hover:text-white' | ||
| }`} | ||
| > | ||
| <div className="font-medium">{cat.name}</div> | ||
| <div className={`text-xs mt-0.5 ${cat.id === currentCategory ? 'text-blue-100' : 'text-gray-500 group-hover:text-gray-400'}`}> | ||
| {cat.description} | ||
| </div> | ||
| </Link> | ||
| ))} |
There was a problem hiding this comment.
The Link component is missing keyboard focus management. When a user changes categories via keyboard navigation, they should have clear focus indication. Consider adding focus:ring styles or ensuring proper focus management when the category changes programmatically.
| <div className={`text-xs mt-0.5 ${cat.id === currentCategory ? 'text-blue-100' : 'text-gray-500 group-hover:text-gray-400'}`}> | ||
| {cat.description} | ||
| </div> |
There was a problem hiding this comment.
The conditional styling reference uses 'group-hover:text-gray-400' but the parent Link element does not have a 'group' class. This means the hover effect for the description text will not work as intended. Either add 'group' to the Link className or remove the group-hover styling.
| currentCategory, | ||
| categoryDescription | ||
| }: ClientJobBoardLayoutProps) { | ||
| const [searchQuery, setSearchQuery] = useState("") |
There was a problem hiding this comment.
The search query state persists when navigating between categories using browser navigation (back/forward buttons or direct URL changes), which could show incorrect search results. The search state is only cleared on Link click, but not when the component receives new props due to navigation. Consider using a useEffect hook to clear the search query when currentCategory changes.
Hello Pavan,
Added support for a search bar and some subtle UI changes.
Let me know if you have any issues.
Thanks,
Kafka