thunder-app / thunder

Thunder - An open-source cross-platform Lemmy client for iOS and Android built with Flutter
https://thunderapp.dev
GNU Affero General Public License v3.0
732 stars 62 forks source link

Feature/search sorting #373

Closed micahmo closed 10 months ago

micahmo commented 10 months ago
  1. Support for sorting communities in search (mentioned in #63).
    • Note that I didn't make this a setting like Post or Comment sorting. Instead, I just remember the current selection for next time.
  2. Support for paging search results (looks like this was only half implemented).

https://github.com/thunder-app/thunder/assets/7417301/a6d2b4d1-7ba1-4473-808a-46557049ae8b

hjiangsu commented 10 months ago

I think it looks good overall! Just one UI thing - should the sort by be after of before the clear button? Which one makes more sense from a UI perspective?

I took a look at the material guidelines but didn't see any examples of this, so I'd like to open up discussion about this: https://m3.material.io/components/search/guidelines

micahmo commented 10 months ago

should the sort by be after of before the clear button?

I put it after because we currently hide the clear button when there is no text. Therefore, if the sort button were before, there would either be a blank space when there's no clear button, or the sort button would move around. I didn't like the sound of either of those.

I guess the other option is to always show the clear button.

hjiangsu commented 10 months ago

You made some good points - so I'll stick with the current way things are implemented!

machinaeZER0 commented 10 months ago

Apologies if this is outside the scope of this PR - should we consider updating the default search sorting? It looks like currently it sorts by Hot by out of the gate, but in my experience that means a lot of less relevant results (with far fewer subscribers) show up before the thing you may have been looking for. Would it make sense to set the default to something equivalent to Top?

micahmo commented 10 months ago

The default sort used to be Top, but @Benjamint22 changed it to Active in #65. Personally, I agree that Top is very useful. While it's nice to surface smaller communities, I usually find I'm searching for the "main" instance of a given community, which is usually the one with the most subscribers. I think having the option to sort (and having it sticky) is good enough, but I'm certainly open to changing the default back to Top as well.

machinaeZER0 commented 10 months ago

Defer to the group (as always) but I like the idea of starting people with Top - feels sort of in line with the idea of changing the default feed view to All over Local, in my brain. In both cases people have the means to tailor their preferences, but feels like it gives people the optimal first-time experience, personally.

micahmo commented 10 months ago

@machinaeZER0 Maybe this would be a good candidate for you to make another PR? 😉 Take a look at where the search page picks up the default sort here:

https://github.com/thunder-app/thunder/blob/45eb2b7e2f613fa5252baa13604affeaa7b7e71d/lib/search/pages/search_page.dart#L47

And where the default sort is defined here:

https://github.com/thunder-app/thunder/blob/c8110c69884259fd194388e35c07072de6f87e9b/lib/utils/constants.dart#L5

You could make a new constant like DEFAULT_SEARCH_SORT_TYPE with value SortType.topAll in the second file, then use that as the default in the first file.

machinaeZER0 commented 10 months ago

Oh jeez, cross-file shenanigans! I'm definitely interested in giving it a shot - I'll try and check this out this weekend!

Benjamint22 commented 10 months ago

if you guys decide to switch it to top, might be worth looking into switching it to top of a shorter time span than all (like year). I don't know for how many people that's the case, but generally, when I search for a community, it's because I want to know which one will result in the greatest number of good posts in my feed, and top all could return dead communities (although I'm not sure how concerning that is 🤷).

I switched it away from the default (hot) to active, but top sounds good too.

machinaeZER0 commented 10 months ago

would that just be SortType.topYear? which file houses all of these options? Either way your reasoning makes sense to me!

machinaeZER0 commented 10 months ago

image @micahmo Is this all I would be changing in this file once I've made the new const in thunder/lib/utils/constants.dart?

Benjamint22 commented 10 months ago

would that just be SortType.topYear? which file houses all of these options?

I think https://github.com/thunder-app/lemmy_api_client/blob/master/lib/src/enums.dart#L49

machinaeZER0 commented 10 months ago

PR #422 has been created 😄 please let me know if I did that right, if anyone has time at some point?