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 Instances #873

Merged
merged 60 commits into from
Feb 1, 2024
Merged

Search Instances #873

merged 60 commits into from
Feb 1, 2024

Conversation

Sjmarf
Copy link
Contributor

@Sjmarf Sjmarf commented Jan 22, 2024

Note

This PR is built ontop of #869. Drafting until that PR is merged.

Instance Search

Adds an "instances" tab to search. When the app opens, the instance list is downloaded from https://github.com/mlemgroup/mlem-stats. That repo has a GH action set up to refresh the data every 24 hours. So if an instance changes their avatar or version number, it could take 48 hours for it to be updated in Mlem (lemmyverse's 24h maximum, followed by our 24h maximum). Instances that have a negative score, have less than 20 users or are censured on fediseer are excluded. This cuts the list down from ~850 to ~160 instances.

The search tab code is a little clunky, and will need to be re-done as a whole at some point. I'll do that post-iOS 17, at the same time I do post searching probably.

What do we do about NSFW instances? There's currently no way to tell if an instance is NSFW-only, although there's a PR open for this.

@Sjmarf Sjmarf self-assigned this Jan 22, 2024
@Sjmarf Sjmarf added this to the v1.2 milestone Jan 29, 2024
@Sjmarf Sjmarf mentioned this pull request Jan 29, 2024
@Sjmarf Sjmarf marked this pull request as ready for review January 31, 2024 18:05
@Sjmarf Sjmarf requested a review from a team as a code owner January 31, 2024 18:05
@Sjmarf Sjmarf requested review from mormaer and EricBAndrews and removed request for a team January 31, 2024 18:05
Copy link
Member

@EricBAndrews EricBAndrews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A couple nits, and I also noticed some inconsistent behavior around searching:

Screen.Recording.2024-01-31.at.4.44.29.PM.mov

It's minor enough that I don't see a need to hold up the merge to resolve it, but if we merge with this behavior still present it's worth opening an issue.

Mlem/Views/Tabs/Search/SearchResultsView.swift Outdated Show resolved Hide resolved
Mlem/Views/Tabs/Search/SearchView.swift Outdated Show resolved Hide resolved
@Sjmarf
Copy link
Contributor Author

Sjmarf commented Jan 31, 2024

Nice catch! Should be fixed now 👍

Copy link
Member

@EricBAndrews EricBAndrews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🚀

@Sjmarf Sjmarf merged commit ccd203a into dev Feb 1, 2024
4 checks passed
@Sjmarf Sjmarf deleted the sjmarf/instance-search branch February 1, 2024 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants