Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search in spaces and in inbox #2294

Closed
5 of 9 tasks
elatif2020 opened this issue Nov 5, 2023 · 17 comments · Fixed by #2390
Closed
5 of 9 tasks

Search in spaces and in inbox #2294

elatif2020 opened this issue Nov 5, 2023 · 17 comments · Fixed by #2390
Labels
Priority - B V [dev] verified on dev enviroment V [production] verified on production enviroment

Comments

@elatif2020
Copy link
Collaborator

elatif2020 commented Nov 5, 2023

See design here

  • Add search button in spaces see here
    Image
    (the background should be white, and gray on hover)
  • Open search line,
  • Make sure to immediately enable text focus in it
  • Once a text is entered filter the list of items according to the title (Edit: without the description)
  • Close the search on a click on the "x" (or if the user switch to another page such as the profile)

Lower priority (Later):

  • Highlight matching text in the title
  • Enable search also in the description
  • Replace the last message preview line with a view of part of the description with the matching text highlighted
  • Similarly add search button in inbox

For now don't show search on mobile (a separate ticket for it - here)

@budnik9
Copy link
Contributor

budnik9 commented Dec 1, 2023

@elatif2020 In the requirements the search in inbox page is marked as later change. But all designs show search feature on the inbox page. Please confirm that the search on the inbox page is a later change

image

If we only implement the search on the space page for now, should we put the search button in the 3dots menu?

@elatif2020
Copy link
Collaborator Author

@elatif2020 In the requirements the search in inbox page is marked as later change. But all designs show search feature on the inbox page. Please confirm that the search on the inbox page is a later change

image

If we only implement the search on the space page for now, should we put the search button in the 3dots menu?

Hi @budnik9 thanks for bringing this up 🙏

  1. Yes search in inbox is indeed a bit lower priority. However it is very similar thus I'm wondering whether it is efficient to split it or better to do both together. What do you think?
  2. Search in space in desktop should appear as a separate button, like in the inbox (so it should be a button to the left of the + and the 3dots which is hidden by the search bar once clicked. In mobile (later) it should be in the 3dots (you can see in the ticket).

@budnik9
Copy link
Contributor

budnik9 commented Dec 1, 2023

@elatif2020 Yes, they are really similar. They have the same UI logic, but may have different filtering logic. So when implementing the space search, I will try to make it as reusable as possible. So it will be easy to reuse the UI logic. And if the filtering logic is also similar, I will implement the search on the inbox page as well.

@budnik9
Copy link
Contributor

budnik9 commented Dec 1, 2023

@elatif2020 Please tell me what behavior is expected in the following cases:

case 1:

  1. User enters a value to the search.
  2. Spaces are filtered by this value.
  3. User clicks on close icon
    Should we leave the filtered list of spaces or should we show the original list of spaces (unfiltered)?

case 2:

  1. User enters a value to the search.
  2. Spaces are filtered by this value.
  3. User goes to another page.
    Should we store the filtering result and show the filtered list of spaces when the user returns?

@elatif2020
Copy link
Collaborator Author

@elatif2020 Please tell me what behavior is expected in the following cases:

case 1:

1. User enters a value to the search.

2. Spaces are filtered by this value.

3. User clicks on close icon
   Should we leave the filtered list of spaces or should we show the original list of spaces (unfiltered)?

case 2:

1. User enters a value to the search.

2. Spaces are filtered by this value.

3. User goes to another page.
   Should we store the filtering result and show the filtered list of spaces when the user returns?

@budnik9
case 1 - yes, closing the search should bring back to the full list
case 2 - might be good to keep this search word as a query parameter in the url, so if a user click back it will bring her to search results (this will be especially important on mobile), but if she come back to this space by e.g. clicking this space from the breadcrumb it will clear the search

@budnik9
Copy link
Contributor

budnik9 commented Dec 4, 2023

@danielr18
Do we have any param for the /commons/:commonId/feed-items endpoint to filter feed items?

@elatif2020
Copy link
Collaborator Author

elatif2020 commented Dec 4, 2023

@danielr18 Do we have any param for the /commons/:commonId/feed-items endpoint to filter feed items?

@budnik9
I guess we don't have.
Do we need it?
Maybe it could be better to have @andreymikhadyuk involved here in the conversation. Previously I was discussing with him the possible implementation of this filtration to be made only from the FE side (I mean by filtering the existing items and fetching more items in parallel)

@budnik9
Copy link
Contributor

budnik9 commented Dec 4, 2023

@elatif2020
If we want to filter among all existing feed items, we need to do it with BE.
We can filter items only on FE side. But in that case we will filter only items that were already loaded. And it is also necessary to choose a loading strategy for next batches of the feed items:

  1. We don't load the next batches until search input is empty/closed
  2. We load the next batches and when these batches are loaded, we will display these feed items without filtering (horrible UX)
  3. We load the next batches, but without considering the filtering value. And when these batches are loaded, we will filter them on the frontend side and display only matching feed items

@danielr18
Copy link
Collaborator

It's not possible at the moment

@elatif2020
Copy link
Collaborator Author

@elatif2020 If we want to filter among all existing feed items, we need to do it with BE. We can filter items only on FE side. But in that case we will filter only items that were already loaded. And it is also necessary to choose a loading strategy for next batches of the feed items:

1. We don't load the next batches until search input is empty/closed

2. We load the next batches and when these batches are loaded, we will display these feed items without filtering (horrible UX)

3. We load the next batches, but without considering the filtering value. And when these batches are loaded, we will filter them on the frontend side and display only matching feed items

@budnik9
I think we should do the 3rd option you mentioned.
But there might be an issue depending on the numbers: if there are additional 50 items in the feed it makes sense (I think) to load all of them, and filter them with the search. But if there are say 500 more items it might be too much, and we might want to load directly only filtered items if possible.

@budnik9
Copy link
Contributor

budnik9 commented Dec 5, 2023

@elatif2020 I won't load all batches. I will load them one by one when the user scrolls to the end of the list (like we do it now)

@elatif2020
Copy link
Collaborator Author

elatif2020 commented Dec 5, 2023

@budnik9 sure
But it could be for example that the user type something that doesn't exist and we will have to load all batches to verify this...

Also we need a placeholder text for this case:
"No matching items have been found yet."
and display our spinner below

And once we are sure there are no items change it to:
"Looks like there are no matches for your query."

@budnik9 budnik9 linked a pull request Dec 11, 2023 that will close this issue
3 tasks
@elatif2020
Copy link
Collaborator Author

@budnik9 I played with the preview
it is great 🙏
comments:

  1. Let's close the search once the plus ➕ button is clicked.
  2. I guess this (from my comment above) isn't implemented: 'if a user click back it will bring her to search results (this will be especially important on mobile), but if she come back to this space by e.g. clicking this space from the breadcrumb it will clear the search'. Is it complicated or simple to add?

@budnik9
Copy link
Contributor

budnik9 commented Dec 12, 2023

@elatif2020

  1. No problem. Will implement this!
  2. I'm not sure I fully understand what you mean. If we need it to save search results for mobile view when user opens and closes chats, let's implement this as a part of mobile search ticket.

@elatif2020
Copy link
Collaborator Author

@budnik9 can you please change the placeholder text to "Search space" (without the s)
And for the inbox it should be of course "Search inbox"

@budnik9
Copy link
Contributor

budnik9 commented Dec 12, 2023

@elatif2020 Done!

pvm-code added a commit that referenced this issue Dec 14, 2023
@NoamQA NoamQA added the V [dev] verified on dev enviroment label Dec 19, 2023
@NoamQA
Copy link
Collaborator

NoamQA commented Dec 26, 2023

@elatif2020
works fine in spaces
for now we don't have a button in inbox?

@NoamQA NoamQA added the V [production] verified on production enviroment label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority - B V [dev] verified on dev enviroment V [production] verified on production enviroment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants