551 feature filtering searches for rooms filter sidebar #564
Conversation
There was a problem hiding this comment.
Looks good! Working well for me locally - just had a few questions on some changes but otherwise should be ready to merge soon! 🥳
PS. Regarding the comments on styling - I think you're good to just delete the commented out styles unless you think there's worth in changing them back?
| export const FreeRoomsAPIContext = createContext<ContextType>({ buildings : [], rooms: {}, roomInfo: { rooms: {} }, onRefresh: () => {} }); |
There was a problem hiding this comment.
Hmm this should be for the mobile app so shouldn't need changes?
| @@ -1,23 +1,25 @@ | |||
| "use client"; | |||
|
|
|||
| import SearchIcon from "@mui/icons-material/Search"; | |||
| import { useMediaQuery } from "@mui/material"; | |||
| import Button from "@mui/material/Button"; | |||
| import { Button, CircularProgress } from "@mui/material"; | |||
There was a problem hiding this comment.
Is CircularProgress used in this file?
| const filters = useSelector(selectFilters); | ||
| const { rooms, isValidating } = useAllRooms(filters); | ||
| const centred = isValidating ? "center" : "default"; | ||
| // const displayMobile = useMediaQuery(useTheme().breakpoints.down("md")); |
There was a problem hiding this comment.
Just checking that this is left here for future use when a mobile design is ready?
| [theme.breakpoints.down("md")]: { | ||
| flexDirection: "column", | ||
| alignItems: "center", | ||
| // alignItems: "center", |
There was a problem hiding this comment.
Do we plan on using the commented out styles later?
| borderWidth: 1, | ||
| borderStyle: "solid", | ||
| // borderWidth: 1, | ||
| // borderStyle: "solid", |
There was a problem hiding this comment.
Are we keeping these styles for any reason?
| borderColor: theme.palette.mode === "light" ? "#BCBCBC" : "#3F3F3F", | ||
| ":hover": { | ||
| cursor: "auto", | ||
| }, | ||
| })); | ||
|
|
||
| const FilterSideBar = () => { | ||
| const FilterSideBar = ({ filters }: { filters: Filters }) => { | ||
| // To do - make this the same component as filter bar? |
There was a problem hiding this comment.
I think for now we keep them separate because in the future it's uncertain how we may want to distinguish functionality
| state.value[key]!.splice(targetIndex); | ||
| } | ||
| } else { | ||
| throw "unsetting unincluded key"; |
There was a problem hiding this comment.
Could you please explain to me how this section works? 😅
In this pull request:
connected all rooms filter sidebar to backend endpoint
made small styling changes to allrooms page layout (removing mobile filter, adding loading state)
changed filter sidebar logic to use regular filter slice
fixed allroomsfilterslice, it is now unused but could be used if we wanted to multi select filters in the future?
made some style changes but commented out in case they need to be added again
tech debt