Location Filter Styling #561
Conversation
- Separate component with css called in ListPage.tsx - Hijacks the current search feature (not its own filter) - Possibly to do: make it work with other incoming filters
- Fragment still needed for some reason in SelectLocation.tsx
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -3,43 +3,46 @@ import { IReadOnlyLocation_FromAPI_PostProcessed } from '../types/locationTypes' | |||
| import './SelectLocation.css'; | |||
|
|
|||
| type SelectLocationProps = { | |||
| SSQ: (value: React.SetStateAction<string>) => void; | |||
| l: IReadOnlyLocation_FromAPI_PostProcessed[] | undefined; | |||
| setSearchQuery: (value: React.SetStateAction<string>) => void; | |||
There was a problem hiding this comment.
React.SetStateAction<string> is a good way of typing this! (I would've done (value:string)=>void but that doesn't fully cover the type)
We can actually replace (value: React.SetStateAction<string>) => void; with React.Dispatch<React.SetStateAction<string>> to be even more semantically correct.
| function SelectLocation({ SSQ, l }: SelectLocationProps) { | ||
| if (l === undefined) { | ||
| function SelectLocation({ setSearchQuery, locations }: SelectLocationProps) { | ||
| if (locations === undefined) { |
There was a problem hiding this comment.
good use of strict equality === checking!
| let locationStrings = locations.map((locationObj) => locationObj.location); | ||
| locationStrings = locationStrings.map((locationObj) => | ||
| locationObj.indexOf(',') === -1 | ||
| ? locationObj | ||
| : locationObj.slice(0, locationObj.indexOf(',')), | ||
| ); |
There was a problem hiding this comment.
I feel like extracting the
? locationObj
: locationObj.slice(0, locationObj.indexOf(','))
into its own outer function with a signature like getPrimaryLocation(locationString:string)=>string and writing const locationStrings = locations.map((locationObj) => getPrimaryLocation(locationObj.location)); would be more clear for future maintenence.
|
|
||
| locationStrings = Array.from(new Set(locationStrings)); |
There was a problem hiding this comment.
How about [...new Set(locationStrings)];? And how about making it more clear what we're doing here by calling this new variable const dedeupedLocationStrings = [...new Set(locationStrings)];`?
And I do agree it might be a bit jank to convert to a set and then back into a list. If you think the other methods outlined here are cleaner, go for it!
| // The whole thing breaks if this useless, extraneous piece is deleted. | ||
| // i don't know why. | ||
| return <>🥥</>; | ||
| return <>BUG: breaks if removed</>; |
There was a problem hiding this comment.
I found the issue! It's a bug with million.vite in vite.config.ts, not with this code. I recommend deleting that and uninstalling million (it's supposed to make react apps more performant, but cmueats is relatively lightweight.) @GhostOf0days let me know if you disagree.
There was a problem hiding this comment.
I do like Million, but yeah, we shouldn't need it for CMUEats. Removing is fine.
|
@cirex-web Wanted your opinion on this, but I do think the UI looks out of place compared to the rest of the UI. Should also change CSS for that. 
|
|
Update: It looks fine on mobile but off on my MacBook. |
|
What browser are you on? |
|
Safari |
|
Will need to do more to add back the dropdwon arrow, but looking into it, it's because Apple forces their version for the select button. To force custom version, add the following (but this leads to dropwdown arrow disappearing, so need to fix that as well maybe with background image, etc. Good resource: https://stackoverflow.com/questions/14218307/select-arrow-style-change): -webkit-appearance: none; |
Description
Made SelectLocation.tsx more readable, better styling
Type of change
How Has This Been Tested?
Test Configuration:
Checklist: