frontend - add clear filter button #5275 #5413

graceegal · 2024-12-30T02:42:40Z

closes #5275
part of #5341

Web frontend checklist

Formatted my code with yarn format
There are no warnings from yarn lint --fix
There are no console warnings when running the app
Added any new components to storybook
Added tests where relevant
All tests pass
Clicked around my changes running locally and it works
Checked Desktop, Mobile and Tablet screen sizes

…ap buttons

vercel · 2024-12-30T02:42:44Z

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

Name	Status	Preview	Updated (UTC)
couchers	✅ Ready (Inspect)	Visit Preview	Jan 11, 2025 9:46pm

nabramow · 2024-12-30T04:16:28Z

Needs some margin around the right side of the button here. I'd make it the same as the distance between search in this area and this.

nabramow · 2024-12-30T04:17:44Z

When I click clear filters, it also clears my selected location. IMO it shouldn't clear location but everything else. But deferring to @jesseallhands what do you think?

nabramow · 2024-12-30T05:10:10Z

app/web/features/search/MapWrapper.tsx

+  /**
+   * Checks if filters are applied
+   */
+  useEffect(() => {
+    const filtersActive =
+      queryName !== "" ||
+      lastActiveFilter !== 0 ||
+      hostingStatusFilter.length !== 0 ||
+      numberOfGuestFilter !== undefined ||
+      completeProfileFilter !== false ||
+      (locationResult?.location.lng !== 0 &&
+        locationResult?.location.lat !== 0);
+
+    setAreFiltersApplied(filtersActive);
+  }, [
+    queryName,
+    lastActiveFilter,
+    hostingStatusFilter,
+    numberOfGuestFilter,
+    completeProfileFilter,
+    locationResult,
+  ]);
+


Okay first of all, this was a great first attempt, and it pretty much works! Well done taking this on as one of your first tickets as honestly the map is confusing even for me!

A few notes on things I'd change due to the approach:

First a caveat - this isn't how I'd ideally do this in i.e. a day job, normally I'd just send an api call to the backend to clear everything and that would return an updated response, which would update the UI.

BUT we cache the result and filter it on the frontend, which is why that doesn't work here.

Given we will refactor the map soon anyway, this is how I'd go about it as a stop gap:

We already have a very similar useEffect to the one you created for wasSearchPerformed in the parent component SearchPage so IMO we want to avoid another one.

Also I'd generally try to keep this functionality in the same place the values are set in useState, which is in SearchPage.

Since you are correctly clearing the values there, we just need a variable to pass down to say "hey the filters changed refilter the user pin results in this component. Your function is already updating the state, which is re-running the api call and regenerating "result", which gets passed down to MapWrapper.

So IMO we just need to tell MapWrapper the search was cleared as the actual functionality is already there.

I'd simplify it a bit like so:

In SearchPage create a useState variable areFiltersCleared, setAreFiltersCleared and set it to true as a default value

When you run the handleClearFilters function in SearchPage, set this to true

In SearchPage, in the useEffect that checks if if (!wasSearchPerformed) create an else to the inner if clause that checks if everything is 0 or undefined, and setAreFiltersCleared false there

In MapWrapper, in the useEffect that re-renders users list on map, change wasSearchedPerformed to {wasSearchPerformed || areFilltersCleared)

I thiiiiink that should do it but let me know if not!

Awesome, thank you so much for the detailed feedback and explanation Nicole! Really appreciate it!

I've made the changes you outlined, however I just want to call out that updating the useEffect that checks if if (!wasSearchPerformed) in SearchPage differed a bit from what you proposed... this was to ensure that the areFiltersCleared state was properly updated all the time, not just upon the first search. LMK if this makes more sense now.

nabramow · 2024-12-30T05:11:44Z

app/web/features/search/MapWrapper.tsx

+   * Clicks on 'clear filters' button
+   */
+  const handleClearFiltersOnClick = () => {
+    handleClearFilters();


Just as a standard practice, I'd change the prop name from handleClearFilters to onClearFiltersClick`.

Generally the handle functions update the state in that same component. If passing down to a child component, name it the action it does instead.

Updated! That makes sense thanks for the info.

bakeiro · 2024-12-30T12:36:47Z

When I click clear filters, it also clears my selected location. IMO it shouldn't clear location but everything else. But deferring to @jesseallhands what do you think?

The thing is, this is not "clear filters" (AFAIK) this is show all the users, and to do that we need to delete everything (including the location) because we can't apply filters/search a location when we display all the users on the map...

jesseallhands · 2024-12-30T12:51:56Z

When I click clear filters, it also clears my selected location. IMO it shouldn't clear location but everything else. But deferring to @jesseallhands what do you think?

It should clear location as well, but not reset the map position.

If we don't clear location we still have the problem that users cannot see all of the pins on the map which is one of the main problems we are trying to resolve with this button.

graceegal · 2024-12-30T21:29:11Z

Needs some margin around the right side of the button here. I'd make it the same as the distance between search in this area and this.

@nabramow hmmm that's weird the border radius is rounded for you. On my vercel preview and on my localhost mine looks like this (with the filter buttons intentionally squashed together but no border radius where they're touching)

UPDATE: I re-ran all of my environments locally and now mine is showing like your screenshot... vercel preview still shows as intended though (weird). I will defer to initial design of equal border radius and margin since it is the most simple fix, unless you say otherwise.

graceegal · 2025-01-02T04:31:54Z

When I click clear filters, it also clears my selected location. IMO it shouldn't clear location but everything else. But deferring to @jesseallhands what do you think?

@nabramow @jesseallhands @bakeiro
updated so that it does reset any location that was set, but does not reset the map position (the current bbox stays the same). LMK if this is the correct functionality yall had in mind!

… the map position

nabramow · 2025-01-03T16:58:12Z

app/web/features/search/MapWrapper.tsx

+  const onClearFiltersClick = () => {
+    const currentBbox = map.current?.getBounds().toArray();
+    if (currentBbox) {
+      const bbox: [number, number, number, number] = [
+        currentBbox[0][0],
+        currentBbox[0][1],
+        currentBbox[1][0],
+        currentBbox[1][1],
+      ];
+      handleClearFilters(bbox);
+    } else {
+      handleClearFilters();
+    }
+  };


Ah sorry it's the other way. Name the original function "handleClearFiltersClick", but when you pass it as a prop, name it 'onClickFiltersClick'.

That's because the function is "handling" it, but when you pass it you want to see the action it's doing.

aha! Okay just updated all of that. Thank you :)

app/web/features/search/MapWrapper.tsx

app/web/features/search/SearchPage.tsx

jesseallhands · 2025-01-03T17:18:58Z

updated so that it does reset any location that was set, but does not reset the map position (the current bbox stays the same). LMK if this is the correct functionality yall had in mind!

Yes, sounds correct!

I tested it on mobile but it doesn't appear to be working. See video below where you can see the "clear filters" button never is clickable despite a location filter (Berlin) being applied. Also, when I click "search here" the button also is not clickable despite the "search here" filter parameter being applied:

Screen_Recording_20250103_110626_Firefox.mp4

nabramow · 2025-01-08T23:11:10Z

app/web/features/search/SearchPage.tsx

@@ -105,7 +107,9 @@ export default function SearchPage({
  >();

  const [isFiltersOpen, setIsFiltersOpen] = useState(false);
-  const [areFiltersCleared, setAreFiltersCleared] = useState(true);
+  const [areFiltersCleared, setAreFiltersCleared] = useState(
+    locationResult.name === ""


What happens if locationResult is not finished loading? It's risky to make one state variable dependent on another in the same component. Curious why not just set it to true by default?

If it is set to true by default then the clear filters button will be disabled on initial render, even if the map is initially rendered with a location filter (ie: someone searched Berlin from the homepage) which is an issue. I see what you mean about the risk of making one state variable dependent on another though... I am thinking to change it to make it dependent on the locationName prop instead. Does that seem like a good fix?

It looks like locationName and bbox are passed in the props from the parent component to SearchPage. I assume that's coming from the dashboard (please check). If so you could use that instead, something like locationName.length > 0 as the default value.

nabramow · 2025-01-08T23:12:47Z

app/web/features/search/SearchPage.tsx

@@ -160,9 +162,9 @@ export default function SearchPage({
      numberOfGuestFilter !== undefined ||
      completeProfileFilter !== false ||
      queryName !== "" ||
-      (locationResult.location.lng !== 0 && locationResult.location.lat !== 0);
+      locationResult.name != "" ||


I think you want locationResult.name !== "" here (two equal signs)

nabramow · 2025-01-08T23:13:24Z

app/web/features/search/SearchPage.tsx

    setQueryName("");
    setLocationResult({
-      bbox: bbox || [0, 0, 0, 0],
+      bbox: [390, 82, -173, -66],


Why these hardcoded numbers?

They are the hardcoded numbers that set the original map position on initial render, so upon clearing filters I set them back to the same numbers to remain consistent

Ahhh gotcha okay!

nabramow · 2025-01-08T23:15:33Z

@nabramow @jesseallhands Clear filters button functionality should now match what we discussed on the meeting today. LMK if not!

A few other things I noticed while working on this and wanted to address:

SearchResults List Mobile View: On mobile only, SearchResultsList is collapsed down and hidden when no filters are applied (this occurs in 2 situations - when they open the map from dashboard without an initial location search query AND every time that clear filters button is clicked). Is that okay behavior or would you prefer it be shown even when no filters are applied?

LocationAutocomplete Sync Issue: When clearing filters and when a location is searched, there's an inconsistency between two instances of LocationAutocomplete (one in SearchBox and one in FilterDialog). The location input might show different values in each place - for example, SearchBox might still display "London" from an initial Dashboard search while FilterDialog shows empty or a different location. Should this synchronization issue be addressed in this PR or as a separate ticket?

Yeah these are both outstanding issues! David is working on 1 in #5294 and 2 I've come across before. Definitely things we should fix, but they existed before this PR so you don't need to address here.

nexagoai · 2025-01-09T17:06:45Z

app/web/features/search/SearchPage.tsx

@@ -81,7 +81,9 @@ export default function SearchPage({
  const isMobile = useMediaQuery(theme.breakpoints.down("md"));

  // State
-  const [wasSearchPerformed, setWasSearchPerformed] = useState(false);
+  const [wasSearchPerformed, setWasSearchPerformed] = useState(


[David here] maybe a boolean it's better than empty string?

I feel like setting the wasSearchPerformed to false can show all the users on the map?

sorry if I review this late, I've been busy lately 😅

@nexagoai locationName !== "" does return a boolean.

nexagoai · 2025-01-09T17:07:28Z

app/web/features/search/SearchPage.tsx

@@ -105,6 +107,9 @@ export default function SearchPage({
  >();

  const [isFiltersOpen, setIsFiltersOpen] = useState(false);
+  const [areFiltersCleared, setAreFiltersCleared] = useState(


Hey David @nexagoai! If it is set to true by default then the clear filters button will be disabled on initial render, even if the map is initially rendered with a location filter (ie: someone searched Berlin from the homepage) which is an issue. That's why it's currently set to a conditional that checks whether it should be true or false

nabramow · 2025-01-11T20:20:18Z

app/web/features/search/MapWrapper.tsx

 }: mapWrapperProps) {
  const { t } = useTranslation([SEARCH]);
  const [areClustersLoaded, setAreClustersLoaded] = useState(false);
  const [isMapStyleLoaded, setIsMapStyleLoaded] = useState(false);
  const [isMapSourceLoaded, setIsMapSourceLoaded] = useState(false);
  const previousResult = usePrevious(selectedResult);
  const classes = useStyles();
+  const theme = useTheme();
+  const isMobile = useMediaQuery(theme.breakpoints.down("sm"));


Suggested change

const isMobile = useMediaQuery(theme.breakpoints.down("sm"));

const isMobile = useMediaQuery(theme.breakpoints.down("md"));

Medium here to represent mobile as it doesn't include the size defined. So this would mean "below medium" rather than "medium and below".

nabramow · 2025-01-11T20:21:19Z

app/web/features/search/MapWrapper.tsx

@@ -1,5 +1,6 @@
 import ReplayIcon from "@mui/icons-material/Replay";
 import TuneIcon from "@mui/icons-material/Tune";
+import { useMediaQuery, useTheme } from "@mui/material";


Rather than useTheme you can just import our customized theme:

import { theme } from "theme";

nabramow · 2025-01-11T20:33:37Z

Okay functionality-wise this is working great for me on both mobile and web. Happy to approve just those two small changes!

graceegal · 2025-01-11T22:10:45Z

Okay functionality-wise this is working great for me on both mobile and web. Happy to approve just those two small changes!

@nabramow Awesome, thanks! I've made the final 2 updates so should be ready for approval. Note- the vercel deployment failed (for what seemed like no reason just a weird hiccup) so the last 2 commits were just to fix it and have it regenerate the vercel preview (I just moved an import 1 line down and then back again because of linting). All good now though and all checks pass :)

nabramow

Looks good to me! Thanks for your persistence on this one! 🚀

This reverts commit a670d24.

graceegal added 3 commits December 24, 2024 10:02

added a clear filters button to map & updated styling/text of other m…

87b7a8c

…ap buttons

Add test for clearFilters button and cleaning up code

fac21c4

Merge branch 'develop' into web/feature/map-clear-filter-button

c9049d0

graceegal added the 1.topic frontend label Dec 30, 2024

graceegal requested a review from a team December 30, 2024 02:42

graceegal self-assigned this Dec 30, 2024

nabramow reviewed Dec 30, 2024

View reviewed changes

graceegal added 4 commits December 30, 2024 13:38

updated prop name for clear filters onClick function

458a10e

simplified and condensed useEffect for managing filter state

6886132

fixed bug that prevented numberOfGuests filter from properly resetting

15a92be

updated button styles

da3bff1

vercel bot deployed to Preview December 30, 2024 23:53 View deployment

updated clear filters so that it resets location but no longer resets…

75890fa

… the map position

vercel bot deployed to Preview January 2, 2025 04:37 View deployment

graceegal requested a review from nabramow January 2, 2025 04:55

nabramow reviewed Jan 3, 2025

View reviewed changes

app/web/features/search/MapWrapper.tsx Outdated Show resolved Hide resolved

nabramow reviewed Jan 3, 2025

View reviewed changes

app/web/features/search/MapWrapper.tsx Outdated Show resolved Hide resolved

nabramow reviewed Jan 3, 2025

View reviewed changes

app/web/features/search/MapWrapper.tsx Outdated Show resolved Hide resolved

nabramow reviewed Jan 3, 2025

View reviewed changes

app/web/features/search/SearchPage.tsx Outdated Show resolved Hide resolved

update prop name for handle clear filters function

603612a

nabramow reviewed Jan 8, 2025

View reviewed changes

graceegal added 2 commits January 8, 2025 16:11

Updated state variable

efde82a

Merge branch 'develop' into web/feature/map-clear-filter-button

15784fd

vercel bot deployed to Preview January 9, 2025 00:19 View deployment

nexagoai reviewed Jan 9, 2025

View reviewed changes

Update test as attempt to fix GitLab check

bc797e7

vercel bot deployed to Preview January 9, 2025 20:57 View deployment

graceegal requested a review from nabramow January 11, 2025 19:32

nabramow reviewed Jan 11, 2025

View reviewed changes

Update theme import and mobile screen breakpoint

41bd2de

vercel bot had a problem deploying to Preview January 11, 2025 20:37 Failure

Attempt to fix Vercel preview deployment

c5f95eb

vercel bot deployed to Preview January 11, 2025 21:14 View deployment

Attempt to fix Vercel preview deployment - part 2

fa60483

vercel bot deployed to Preview January 11, 2025 21:46 View deployment

graceegal requested a review from nabramow January 11, 2025 22:11

nabramow approved these changes Jan 12, 2025

View reviewed changes

graceegal merged commit a670d24 into develop Jan 13, 2025
3 checks passed

graceegal deleted the web/feature/map-clear-filter-button branch January 13, 2025 18:19

nabramow added a commit that referenced this pull request Jan 15, 2025

Revert "frontend - add clear filter button #5275 (#5413)"

d803013

This reverts commit a670d24.

This was referenced Jan 15, 2025

Revert "frontend - add clear filter button #5275" #5475

Merged

Bug fix - Map not showing full users after clear with large data set #5474

Closed

nabramow linked an issue Jan 15, 2025 that may be closed by this pull request

clear filter of the map button #5078

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

frontend - add clear filter button #5275 #5413

frontend - add clear filter button #5275 #5413

graceegal commented Dec 30, 2024 •

edited

Loading

vercel bot commented Dec 30, 2024 •

edited

Loading

nabramow commented Dec 30, 2024

nabramow commented Dec 30, 2024

nabramow Dec 30, 2024

graceegal Dec 30, 2024

nabramow Dec 30, 2024

graceegal Dec 30, 2024

bakeiro commented Dec 30, 2024 •

edited

Loading

jesseallhands commented Dec 30, 2024

graceegal commented Dec 30, 2024 •

edited

Loading

graceegal commented Jan 2, 2025

nabramow Jan 3, 2025

graceegal Jan 3, 2025

jesseallhands commented Jan 3, 2025

nabramow Jan 8, 2025

graceegal Jan 9, 2025 •

edited

Loading

nabramow Jan 9, 2025

nabramow Jan 8, 2025

nabramow Jan 8, 2025

graceegal Jan 9, 2025

nabramow Jan 9, 2025

nabramow commented Jan 8, 2025

nexagoai Jan 9, 2025 •

edited

Loading

nexagoai Jan 9, 2025 •

edited

Loading

nexagoai Jan 9, 2025

nabramow Jan 9, 2025

nexagoai Jan 9, 2025

graceegal Jan 9, 2025

nabramow Jan 11, 2025

nabramow Jan 11, 2025

nabramow commented Jan 11, 2025

graceegal commented Jan 11, 2025 •

edited

Loading

nabramow left a comment

	const isMobile = useMediaQuery(theme.breakpoints.down("sm"));
	const isMobile = useMediaQuery(theme.breakpoints.down("md"));

frontend - add clear filter button #5275 #5413

frontend - add clear filter button #5275 #5413

Conversation

graceegal commented Dec 30, 2024 • edited Loading

vercel bot commented Dec 30, 2024 • edited Loading

nabramow commented Dec 30, 2024

nabramow commented Dec 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bakeiro commented Dec 30, 2024 • edited Loading

jesseallhands commented Dec 30, 2024

graceegal commented Dec 30, 2024 • edited Loading

graceegal commented Jan 2, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jesseallhands commented Jan 3, 2025

Choose a reason for hiding this comment

graceegal Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabramow commented Jan 8, 2025

nexagoai Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

nexagoai Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabramow commented Jan 11, 2025

graceegal commented Jan 11, 2025 • edited Loading

nabramow left a comment

Choose a reason for hiding this comment

graceegal commented Dec 30, 2024 •

edited

Loading

vercel bot commented Dec 30, 2024 •

edited

Loading

bakeiro commented Dec 30, 2024 •

edited

Loading

graceegal commented Dec 30, 2024 •

edited

Loading

graceegal Jan 9, 2025 •

edited

Loading

nexagoai Jan 9, 2025 •

edited

Loading

nexagoai Jan 9, 2025 •

edited

Loading

graceegal commented Jan 11, 2025 •

edited

Loading