frontend - feat: add new list view of users on map #4848 #5294

marco-digennaro · 2024-12-04T18:06:59Z

Web frontend checklist

[ x] Formatted my code with yarn format
[x ] 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
[ xx] Clicked around my changes running locally and it works
Checked Desktop, Mobile and Tablet screen sizes

vercel · 2024-12-04T18:07:04Z

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

Name	Status	Preview	Updated (UTC)
couchers	✅ Ready (Inspect)	Visit Preview	Jan 2, 2025 0:12am

jesseallhands · 2024-12-04T19:12:16Z

@marco-digennaro this is looking really good! I'm super excited to this feature to be rolled out as is should improve mobile QoL by a lot.

Here are a few things I noticed in the preview (from a UI/UX perspective, I'm not a FE dev):

The spacing between the cards is too much and takes up a lot of screen real estate:

When the user selects a single pin, it should not show the expand drawer arrow ^ because there is no point of expanding the drawer upward for a single user card. Also, the spacing above and below the card is too much:

When no pin is selected, the drawer still appears, which should not happen:

After clicking on a pin, if the user zooms in and clicks on "search in this area" the card for the previously clicked pin persists, even if that pin is no longer in the search area:
https://github.com/user-attachments/assets/e32ba8e2-1ccf-444a-81c8-49f5864811c6

It's possible some of the map behavior issues may be caused by the next instance, but I'm not sure. Perhaps @nabramow and/or @darrenvong can assist here in helping assess the causes.

bakeiro · 2024-12-04T20:33:42Z

this is so cool feature 🤩 , thanks for this!

bakeiro · 2024-12-04T20:35:30Z

code looks great, once the UI feedback is done, and the build isn't red I can re-check and most likely approve :)

app/web/features/search/SearchResultsList.tsx

nabramow · 2024-12-05T01:19:11Z

app/web/features/search/SearchResultsMobileVerticalList.tsx

+const useStyles = makeStyles((theme) => ({
+  drawer: {
+    /*  */
+    width: "100%",
+    height: "350px",
+    overflowY: "auto",
+    "&[data-open-state='true']": {
+      height: "100%",
+      top: "56px",
+      position: "fixed",
+      bottom: 0,
+      left: 0,
+      zIndex: theme.zIndex.drawer,
+      backgroundColor: theme.palette.background.default,
+    },
+  },
+  singleResult: {
+    width: "100%",
+    display: "flex",
+    flexDirection: "column",
+    justifyContent: "center",
+    alignItems: "center",
+    maxHeight: "400px",
+    paddingBottom: theme.spacing(2),
+    overflowY: "auto",
+  },
+  verticalList: {
+    display: "flex",
+    flexDirection: "column",
+    gap: theme.spacing(2),
+    padding: theme.spacing(2),
+    "& .MuiCard-root": {
+      height: "auto",
+      width: "100%",
+      [theme.breakpoints.down("md")]: {
+        height: "auto",
+        "& .MuiCardContent-root": {
+          height: "auto",
+        },
+        "& .MuiCardActionArea-root": {
+          height: "100%",
+        },
+      },
+    },
+  },
+  openButton: {
+    width: "100%",
+    marginLeft: 0,
+  },
+  closeButton: {
+    width: "100%",
+    marginLeft: 0,
+    position: "sticky",
+    top: 0,
+    backgroundColor: theme.palette.background.default,
+    borderRadius: 0,
+    zIndex: theme.zIndex.drawer,
+  },
+  icon: {
+    fontSize: "3rem",
+  },
+}));


Since this is a new component, could you please use the new MUI styled way of doing the CSS? We're slowly trying to remove all of this way as it's not compatible with the next MUI version.

Just search "Styled" for some examples or you can look at: app/web/components/Footer/Footer.tsx.

marco-digennaro · 2024-12-05T18:39:25Z

It's possible some of the map behavior issues may be caused by the next instance, but I'm not sure. Perhaps @nabramow and/or @darrenvong can assist here in helping assess the causes.

yes, i tried to investigate, it's cause of this piece of code
` const wasResultFound =
resultsList?.find(
(value) => value.user?.userId === selectedResult?.userId
) !== undefined;

if (!wasResultFound && selectedUserData.data) {
resultsList = [{ user: selectedUserData.data, rank: 0, snippet: "" }];
}`
which persist che selected user in the search result
@nabramow is it necessary?

marco-digennaro · 2024-12-05T18:42:11Z

@bakeiro @nabramow @jesseallhands made some adjustments, please check :)

jesseallhands · 2024-12-05T20:24:55Z

The spacing issues are completely resolved! Thanks!

While testing, I ran into an issue where when I clicked on certain pins, it would not center the card within the drawer (it would land somewhere in between cards) see video below:

Screen_Recording_20241205_200746_Firefox_1.mp4

There is also some strange behavior when drawer initially opens. It's almost as if it is showing my own card (and the Chevron for the drawer) just before opening or just before closing. See video:

Screen_Recording_20241205_201332_Firefox.mp4

In addition to resolving that problem, I would also recommend using mountOnEnter and unmountOnExit so the drawer slides in from the bottom upward, which also hints to the user that it is a slidable drawer. I think the same can be used to slide down the drawer when minimizing from the "full screen" list view to the Single-Card view and also when closing the drawer altogether.

Thanks once again for working on this feature! I'm very excited about it and I think mobile users will really love it!

bakeiro · 2024-12-05T20:46:33Z

Screen_Recording_20241205_201332_Firefox.mp4
In addition to resolving that problem, I would also recommend using mountOnEnter and unmountOnExit so the drawer slides in from the bottom upward, which also hints to the user that it is a slidable drawer. I think the same can be used to slide down the drawer when minimizing from the "full screen" list view to the Single-Card view and also when closing the drawer altogether.

Although I agree on this I am not sure how we can solve this as first it is nothing new, it is something old [and in production already....]

The behavior we see on the video is the difference between openning a result which is on the loaded list vs opening a result which isn't in this list, (you may wonder which list is the one on the left in the desktop version).

the background for this is, at the beginning I implemented to show only a subset of results, plus the pagination, what happened is that we wanted to implement to show all users at the first render, because it was something that sold very well.

Since loading all users in the list isn't possible (cause it's to big) therefore, when you click on a user, if this is within the list, it moves and puts the user in the center, otherwise it clears the list temporarily and shows specifically that particular user.

Edit:
I thinking about 2 possible solutions:

Not to show a list at the beginning, in the first render, then whenever you click on a pin, it shows that particular user.
And once you do a search then it shows that result in the list.
Use pagination, and display on the map only what's on the list, plus to prevent the map seeing empty, display the user's surroundings (without much zoom).

apart of this we can create a investigation ticket to find a better solution

marco-digennaro · 2024-12-06T09:19:47Z

There is also some strange behavior when drawer initially opens. It's almost as if it is showing my own card (and the Chevron for the drawer) just before opening or just before closing. See video:

i think there's a different behaviour of the map between mobile and desktop, and apparently i can't reproduce that bug on desktop browser with mobile view.
if i understood the problem on mobile the results change on map move (?), if so i added a (maybe) fix, but again, i couldn't reproduce it so i hope it would work.

PS: where can i find the vercel previews?

bakeiro · 2024-12-06T09:46:29Z

There is also some strange behavior when drawer initially opens. It's almost as if it is showing my own card (and the Chevron for the drawer) just before opening or just before closing. See video:

i think there's a different behaviour of the map between mobile and desktop, and apparently i can't reproduce that bug on desktop browser with mobile view. if i understood the problem on mobile the results change on map move (?), if so i added a (maybe) fix, but again, i couldn't reproduce it so i hope it would work.

PS: where can i find the vercel previews?

Vercel preview :)

marco-digennaro · 2024-12-06T11:01:41Z

There is also some strange behavior when drawer initially opens. It's almost as if it is showing my own card (and the Chevron for the drawer) just before opening or just before closing. See video:

i think there's a different behaviour of the map between mobile and desktop, and apparently i can't reproduce that bug on desktop browser with mobile view. if i understood the problem on mobile the results change on map move (?), if so i added a (maybe) fix, but again, i couldn't reproduce it so i hope it would work.

PS: where can i find the vercel previews?

it didnt work, i need to investigate further why it happens, or if @bakeiro knows already where to look, please write me so i'll try to fix :)

bakeiro · 2024-12-06T14:02:12Z

There is also some strange behavior when drawer initially opens. It's almost as if it is showing my own card (and the Chevron for the drawer) just before opening or just before closing. See video:

i think there's a different behaviour of the map between mobile and desktop, and apparently i can't reproduce that bug on desktop browser with mobile view. if i understood the problem on mobile the results change on map move (?), if so i added a (maybe) fix, but again, i couldn't reproduce it so i hope it would work.
PS: where can i find the vercel previews?

it didnt work, i need to investigate further why it happens, or if @bakeiro knows already where to look, please write me so i'll try to fix :)

I believe that both issues can be solved if we solve the list issue, as I say before depends how we want to solve it

One solution could be not to show a list at the beginning, then when you click on a pin, it shows that particular user
Use Pagination + subsets, and in the first render, show the user surrondings, plus don't allow the user to see the map zommed out

if other person has other suggestion/solution please say, I personally think the best one it's to do the first solution [not show the list at the beginning] and then create a re-search ticket for the future.

If this solution is outside the scope of this PR/ticket I can create another ticket for this :) [then we can merge both PRs together, or you can do a PR from this PR as you prefer...]

write me on Slack if you need more info about this!

bakeiro · 2024-12-06T14:57:04Z

Update: Marco and I talked about this, our plan it's to merge this PR (unless a new bug is found, code suggestions or whatever) and then we will do pair programming to see how to address these bugs [in another PR/ticket]

bakeiro · 2024-12-15T14:43:06Z

to give context:

this is not related with this PR, will be solved once this gets merged :)

bakeiro · 2024-12-15T14:44:58Z

app/web/features/search/SearchPage.tsx

@@ -174,52 +172,35 @@ export default function SearchPage({
    locationResult.location.lat,
  ]);

+  /**


fixes the bug when a selected result remains even after searching

bakeiro · 2024-12-15T14:45:33Z

app/web/features/search/SearchResultsList.tsx

@@ -123,6 +128,10 @@ export default function SearchResultsList({
    .flatMap((page) => page.resultsList)
    .filter((result) => result.user);

+  if (isMobile && !wasSearchPerformed && !searchFromDashboard) {


fix blinking

nabramow

Okay two things here:

The up and down arrows are really big, thick and have a lot of padding around them. That seems non-standard to me. Especially since mobile space is limited, could you make the arrow smaller and less pixels of padding above and below?
Can we just always show this version on mobile even if there is only one result? It seems confusing that it switches when there's more results. I think it's fine if it shows even with one result for consistency.

Otherwise works great functionality-wise so just those 2 design things!

bakeiro · 2024-12-29T05:48:23Z

Can we just always show this version on mobile even if there is only one result? It seems confusing that it switches when there's more results. I think it's fine if it shows even with one result for consistency.

@nabramow

I totally agree here, the problem is that we show all the users the first time (bc is a big selling point apparently) and this creates issues, since we need to change the behavior after the user performs a search/applies a filter, I've been complaining about this during long time, I've created this ticket to explore different solutions #5348

we can talk about this (and we should) but is basically this switch between the first render and after the search which causes a lot of issues, but apparently is better this behavior inconsistency than not showing all the users at the first render...

bakeiro · 2024-12-29T13:39:24Z

I'm thinking about a solution for this... I came up with this idea:
Clicking a result on the map will always show a small bubble on the botton, regardless we display all the users on the map or only a subset, to avoid the weird behaviour, and we display the box with the arrow to toggle the users list only when there is a search/filter (aka subset)

I feel this way it's better than re-locating the list on the button to display the selected user... [basically something like airbnb is doing!]

more info #5405

bakeiro · 2024-12-29T14:56:44Z

small CSS change pushed :)

nabramow · 2024-12-30T04:08:31Z

@bakeiro This looks way better, thanks for making that change! Way more consistent this way.

Just that one comment about the arrow then, would it be possible to make the arrow a bit smaller? Especially the up arrow seems huge and on mobile it's usually thinner and smaller.

bakeiro · 2025-01-02T12:10:01Z

making arrow smaller again, please, let me know if its good enough now @nabramow :)

frontend - feat: add new list view of users on map #4848

0b1bab0

marco-digennaro requested review from bakeiro, nabramow and aapeliv December 4, 2024 18:07

marco-digennaro added the 1.topic frontend label Dec 4, 2024

vercel bot deployed to Preview December 4, 2024 18:10 View deployment

Merge branch 'develop' into add-new-list-view-of-users-on-map#4848

3a9bea6

vercel bot deployed to Preview December 4, 2024 18:17 View deployment

nabramow reviewed Dec 5, 2024

View reviewed changes

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

nabramow reviewed Dec 5, 2024

View reviewed changes

fix: some ux issues, rework style with nmew styled from mui

87bfa29

vercel bot deployed to Preview December 5, 2024 18:43 View deployment

fix: fine tuning on open-close dawer button

a92b4af

vercel bot deployed to Preview December 6, 2024 09:16 View deployment

fix: format and lint

89cea8c

vercel bot deployed to Preview December 6, 2024 10:29 View deployment

bakeiro previously approved these changes Dec 6, 2024

View reviewed changes

vercel bot deployed to Preview December 14, 2024 20:24 View deployment

lint

98bba62

vercel bot deployed to Preview December 14, 2024 21:11 View deployment

fix test

75587b0

vercel bot deployed to Preview December 14, 2024 22:03 View deployment

minor changes

251db12

vercel bot deployed to Preview December 15, 2024 09:40 View deployment

bakeiro mentioned this pull request Dec 15, 2024

Map tweaks #5348

Open

format

b9dc3d3

vercel bot deployed to Preview December 15, 2024 13:40 View deployment

bakeiro requested review from nabramow and bakeiro December 15, 2024 14:44

bakeiro reviewed Dec 15, 2024

View reviewed changes

bakeiro self-requested a review December 15, 2024 14:57

nabramow requested changes Dec 22, 2024

View reviewed changes

nabramow unassigned marco-digennaro Dec 22, 2024

bakeiro mentioned this pull request Dec 29, 2024

[mobile] Map improvements #5405

Open

minor CSS changes

aec81fc

vercel bot deployed to Preview December 29, 2024 14:59 View deployment

nabramow mentioned this pull request Dec 30, 2024

Search doesn't present a list of hosts / search function on mobile is broken #5091

Closed

bakeiro requested a review from nabramow January 2, 2025 12:08

Update SearchResultsMobileVerticalList.tsx

8e1c488

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

frontend - feat: add new list view of users on map #4848 #5294

frontend - feat: add new list view of users on map #4848 #5294

marco-digennaro commented Dec 4, 2024 •

edited by bakeiro

Loading

vercel bot commented Dec 4, 2024 •

edited

Loading

jesseallhands commented Dec 4, 2024

bakeiro commented Dec 4, 2024

bakeiro commented Dec 4, 2024

nabramow Dec 5, 2024

marco-digennaro commented Dec 5, 2024

marco-digennaro commented Dec 5, 2024

jesseallhands commented Dec 5, 2024 •

edited

Loading

bakeiro commented Dec 5, 2024 •

edited

Loading

marco-digennaro commented Dec 6, 2024

bakeiro commented Dec 6, 2024

marco-digennaro commented Dec 6, 2024

bakeiro commented Dec 6, 2024 •

edited

Loading

bakeiro commented Dec 6, 2024 •

edited

Loading

bakeiro commented Dec 15, 2024 •

edited

Loading

bakeiro Dec 15, 2024

bakeiro Dec 15, 2024

nabramow left a comment

bakeiro commented Dec 29, 2024

bakeiro commented Dec 29, 2024 •

edited

Loading

bakeiro commented Dec 29, 2024 •

edited

Loading

nabramow commented Dec 30, 2024

bakeiro commented Jan 2, 2025

frontend - feat: add new list view of users on map #4848 #5294

Are you sure you want to change the base?

frontend - feat: add new list view of users on map #4848 #5294

Conversation

marco-digennaro commented Dec 4, 2024 • edited by bakeiro Loading

vercel bot commented Dec 4, 2024 • edited Loading

jesseallhands commented Dec 4, 2024

bakeiro commented Dec 4, 2024

bakeiro commented Dec 4, 2024

nabramow Dec 5, 2024

Choose a reason for hiding this comment

marco-digennaro commented Dec 5, 2024

marco-digennaro commented Dec 5, 2024

jesseallhands commented Dec 5, 2024 • edited Loading

bakeiro commented Dec 5, 2024 • edited Loading

marco-digennaro commented Dec 6, 2024

bakeiro commented Dec 6, 2024

marco-digennaro commented Dec 6, 2024

bakeiro commented Dec 6, 2024 • edited Loading

bakeiro commented Dec 6, 2024 • edited Loading

bakeiro commented Dec 15, 2024 • edited Loading

bakeiro Dec 15, 2024

Choose a reason for hiding this comment

bakeiro Dec 15, 2024

Choose a reason for hiding this comment

nabramow left a comment

Choose a reason for hiding this comment

bakeiro commented Dec 29, 2024

bakeiro commented Dec 29, 2024 • edited Loading

bakeiro commented Dec 29, 2024 • edited Loading

nabramow commented Dec 30, 2024

bakeiro commented Jan 2, 2025

marco-digennaro commented Dec 4, 2024 •

edited by bakeiro

Loading

vercel bot commented Dec 4, 2024 •

edited

Loading

jesseallhands commented Dec 5, 2024 •

edited

Loading

bakeiro commented Dec 5, 2024 •

edited

Loading

bakeiro commented Dec 6, 2024 •

edited

Loading

bakeiro commented Dec 6, 2024 •

edited

Loading

bakeiro commented Dec 15, 2024 •

edited

Loading

bakeiro commented Dec 29, 2024 •

edited

Loading

bakeiro commented Dec 29, 2024 •

edited

Loading