-
Notifications
You must be signed in to change notification settings - Fork 1
Header search bar now works #150
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useState, useMemo, useEffect } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useNavigate } from 'react-router-dom'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useNavigate, useSearchParams } from 'react-router-dom'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Box, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Container, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -122,6 +122,13 @@ function EventsPage() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [sortBy, setSortBy] = useState<string>('date'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [filterCategory, setFilterCategory] = useState<string>('all'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [searchQuery, setSearchQuery] = useState<string>(''); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [searchParams, setSearchParams] = useSearchParams(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Initialize search query from URL param on mount / param change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const qp = searchParams.get('search') || ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setSearchQuery(qp); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [searchParams]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Get min and max prices from events | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { minPrice, maxPrice } = useMemo(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -262,7 +269,17 @@ function EventsPage() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label="Search events" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| variant="outlined" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={searchQuery} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={(e) => setSearchQuery(e.target.value)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={(e) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const val = e.target.value; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setSearchQuery(val); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setSearchQuery(val); |
Copilot
AI
Nov 22, 2025
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.
There's an inconsistency in how search parameters are handled between Header and EventsPage. The Header trims the search value before encoding it in the URL (trimmed = headerSearch.trim()), but EventsPage sets the URL parameter with the untrimmed value (setSearchParams({ search: val })).
This inconsistency could lead to unexpected behavior. For example, if a user types " test " (with spaces) in EventsPage, the URL will contain the spaces, but if they search from the Header with " test ", the URL will contain "test" without spaces.
Consider trimming the value before setting the search params: setSearchParams({ search: val.trim() })
| setSearchParams({ search: val }); | |
| setSearchParams({ search: val.trim() }); |
Copilot
AI
Nov 22, 2025
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.
When clearing the search parameter, the code mutates the existing searchParams object by calling searchParams.delete('search') before passing it to setSearchParams. While this works, it's modifying a URLSearchParams object that React manages.
A cleaner approach would be to create a new URLSearchParams object:
const newParams = new URLSearchParams(searchParams);
newParams.delete('search');
setSearchParams(newParams);or simply use setSearchParams({}) to clear all params, or avoid the mutation pattern.
| searchParams.delete('search'); | |
| setSearchParams(searchParams); | |
| const newParams = new URLSearchParams(searchParams); | |
| newParams.delete('search'); | |
| setSearchParams(newParams); |
Copilot
AI
Nov 22, 2025
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.
The search input updates URL parameters on every keystroke, which creates a new browser history entry for each character typed. This makes it difficult for users to use the browser's back button effectively and can cause performance issues.
Consider updating the URL only when the user finishes typing (using a debounce) or when they submit the search (blur event or Enter key). The Header.tsx implementation handles this better by only updating on form submission.
| const val = e.target.value; | |
| setSearchQuery(val); | |
| if (val.trim().length > 0) { | |
| setSearchParams({ search: val }); | |
| } else { | |
| // Remove param if empty | |
| searchParams.delete('search'); | |
| setSearchParams(searchParams); | |
| } | |
| }} | |
| placeholder="Search by title, description, or location..." | |
| /> | |
| setSearchQuery(e.target.value); | |
| }} | |
| onBlur={() => { | |
| if (searchQuery.trim().length > 0) { | |
| setSearchParams({ search: searchQuery }); | |
| } else { | |
| // Remove param if empty | |
| searchParams.delete('search'); | |
| setSearchParams(searchParams); | |
| } | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter') { | |
| if (searchQuery.trim().length > 0) { | |
| setSearchParams({ search: searchQuery }); | |
| } else { | |
| searchParams.delete('search'); | |
| setSearchParams(searchParams); | |
| } | |
| } | |
| }} | |
| placeholder="Search by title, description, or location..." |
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.
The Header's
headerSearchstate is not synchronized with the URL search parameter. When a user navigates to the events page with a search query (e.g., from a bookmark or direct link to/events?search=test), the Header's search input will be empty instead of showing "test".Consider using
useSearchParamsand auseEffectin the Header component to sync theheaderSearchstate with the URL parameter when the location changes, similar to how it's implemented in EventsPage.tsx (lines 125-131).