Improved Search #566
Improved Search #566
Conversation
Sjmarf
requested review from
mormaer and
EricBAndrews
and removed request for
a team
| // @State var pinned: Bool = false | ||
|
|
||
| var body: some View { | ||
| Group { | ||
| HStack { | ||
| Text(title) | ||
| .font(.title2) | ||
| .fontWeight(.semibold) | ||
|
|
||
| Spacer() | ||
| if searchModel.activeTypeFilter == nil { | ||
| Button("See All") { | ||
| searchModel.addFilter(filter) | ||
| } | ||
| .foregroundStyle(Color.accentColor) | ||
| .buttonStyle(.borderless) | ||
| } | ||
| } | ||
| .padding(.horizontal, 20) | ||
| .padding(.vertical, 6) | ||
| // .background( | ||
| // ZStack { | ||
| // GeometryReader { | ||
| // Color.clear.preference(key: ViewOffsetKey.self, value: $0.frame(in: .named("searchArea")).origin.y) | ||
| // } | ||
| // if pinned { | ||
| // Rectangle() | ||
| // .fill(.bar) | ||
| // .overlay(alignment: .top) { | ||
| // Divider() | ||
| // } | ||
| // } | ||
| // } | ||
| // ) | ||
| .foregroundStyle(.primary) | ||
| // .onPreferenceChange(ViewOffsetKey.self) { | ||
| // // verify if position is zero (pinned) in container coordinates | ||
| // self.pinned = Int($0) <= 0 | ||
| // } |
| } | ||
| } | ||
|
|
||
| class SearchModel: ObservableObject { |
There was a problem hiding this comment.
nit
This is a model, so it shouldn't live under the Views tree
| content | ||
| .background(Color(UIColor.systemGroupedBackground)) | ||
| .handleLemmyViews() | ||
| // .navigationBarColor() |
There was a problem hiding this comment.
nit
Dead code? Forgot to re-enable?
| ) | ||
| .padding(.bottom, 6) | ||
| } | ||
| LazyVStack(spacing: 10) { // pinnedViews: [.sectionHeaders] |
| // if searchModel.activeTypeFilter != nil { | ||
| // ProgressView() | ||
| // .padding(.top, 15) | ||
| // } | ||
| Spacer() | ||
| .frame(height: 40) | ||
| // .background( | ||
| // GeometryReader { | ||
| // Color.clear.preference(key: ViewOffsetKey.self, value: $0.frame(in: .named("searchArea")).origin.y) | ||
| // | ||
| // } | ||
| // ) | ||
| // .onPreferenceChange(ViewOffsetKey.self) { | ||
| // if $0 < outerGeometry.size.height + 200 { | ||
| // if searchModel.loadedPage >= 1 && searchModel.activeTypeFilter != nil && searchModel.page != searchModel.loadedPage + 1 { | ||
| // searchModel.page = searchModel.loadedPage + 1 | ||
| // } | ||
| // } | ||
| // } | ||
| // .task(id: searchModel.page, priority: .userInitiated) { | ||
| // print("TASK", searchModel.page, searchModel.loadedPage) | ||
| // do { | ||
| // try await searchModel.loadMore() | ||
| // } catch { | ||
| // errorHandler.handle(error) | ||
| // } | ||
| // } | ||
| } |
| // if !atTopOfScrollView { | ||
| // Divider() | ||
| // } | ||
| } |
|
Scrolling performance on searches is also pretty wonky. If you get a bunch of results and scroll, it gets very inconsistent in what posts get rendered--you can scroll for a long time down, then back up super fast. I've also got a few design thoughts I've opened up in the Slack. Overall looks really good though! |
Yep, I'm aware :) As I mentioned, there's still some stuff I needed to fix. Some of that is code that sort-of works for loading more pages, but is too buggy atm. I'll remove/uncomment it before this PR is ready to merge.
Sure 👍 |
Yeah. I have a feeling this is caused by the fancy highlighted-text, which does its calculations using .onAppear. Maybe it's not worth using fancy highlighting. |
|
I haven't had time to review this yet but I notice in the description there is mention of searching instances - would instance search make sense in this area? The user has selected an instance by this stage, we are looking to add some search functionality on instances but I believe this would be restricted to the onboarding / guest journeys before entering the app at which point they have access to this search which is using the current instances API to perform searches for communities/posts/users/comments etc? |
Of course, it's the least likely category to want to search. It would be all the way at the bottom below every other category. When we implement instance pages, it would make sense to include information about how many users the instance has, as well as potentially an uptime graph if we can find a third-party API for it. If we end up having functionality like that, it's reasonable that some people might want to see this information for instances that they aren't a part of. Hence the search-ability. But yeah, not a priority. |
I couldn't picture what would happen when the user tapped an instance result, but general information about it makes sense... once guest mode is in we could let people enter guest mode for the instance from that page, too 😱 |
Indeed! Maybe a "sign up" button could go on the instance page if the user is in guest mode too? The instance sidebar is another bit of information that we don't currently display anywhere, so obviously that'd go in the instance page too. Also an admin list |
|
Will be replaced by an upcoming PR |
Changed the search tab to allow searching of users and posts. This isn't quite done yet, but I wanted to gather some feedback on what I've got so far.
The time results take to load seems to vary; sometimes it can be a little slow. If anyone knows of a way I can speed this up (I'm not too familiar with async stuff), that would be greatly appreciated :)
Things I still need to do: