search

jessicareinh / capstone-event-app

https://capstone-event-app.vercel.app
3 stars 1 forks source link

Category and City Filter #49

Closed thanhtran-git closed 5 months ago

thanhtran-git commented 5 months ago

Update 29.02.24

vercel[bot] commented 5 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
capstone-event-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 29, 2024 11:27am
thanhtran-git commented 5 months ago

Hi team! Great feature, the code looks really good!

One thing that I noticed: When clicking on a category on the categories page and afterwards changing the city, the events are not updated. I have to click on the category again to trigger the fetch of the events. I tried to figure out where the mistake is, but i am having a hard time, because i don't have your process.env keys :D Maybe you can figure it out on your own, otherwise I can give it another go. Maybe the handleCityChange needs to call fetchData(), or something is not right wiht the dependency array of the useEffect.

Other than that, the code looks super good! Let me know if you need any more assistance!

Thanks for your input and quality assurance 😁 The iteration before this one indeed had the handleCityChange fetch new data, just like it is now on the "/" homepage.

I thought it could be an unexpected behavior to load new data on city change, because it is expected to load on category button press. IMO the problem is UI doesn't help the user to let him/her know that he has to change the city first, then press a category button to get new data. I would have to add more conditional rendering of info text, which is not so fun.

But loading new data on city change is also viable, but then there should be an indicator that the user is still in a selected category, like a colored border around the current category button. I will test some solutions now.

thanhtran-git commented 5 months ago

What I have noticed is: If wanted to mark the current category with a border for example, so that a city change triggers loading new data in exactly that category, I would have to save the category as state in the _app.js. That way the city change has access to the current category to fetch new data for that category. My concern is, I already have 9 states in the app.js. How many are too many?

Otherwise, I can implement that a city change fetches new events from all categories, just like on the homepage.

I'm not sure what the best solution is.

somaSeb commented 5 months ago

Filters provide a nice way to drill down on events Reusable components will simplify adding other filters Pages adapted well to handle new filter props

Really nice enhancement overall!