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

Users can edit list of instances to block #372

Merged
merged 6 commits into from
Aug 1, 2023

Conversation

mykdavies
Copy link
Contributor

Really simple implementation to make it available and find out how people really want to use it. Read the comment for possible issues.

@mykdavies mykdavies linked an issue Jul 12, 2023 that may be closed by this pull request
@mykdavies
Copy link
Contributor Author

Screenshot for you visual thinkers:
A simple setting

@zachatrocity
Copy link
Collaborator

This seems like a lot of code and a lot of manual intervention for a feature not a ton of people have asked for.

I feel like this a should be implemented in the backend rather in our third party app.

@mykdavies
Copy link
Contributor Author

mykdavies commented Jul 13, 2023

Yeah, I'd like to have it here on hand in case lemmynsfw really does have an unaddressed issue with putting unmarked content out. Most of the code is about getting the settings page to work tbh. And it is indeed a stopgap until the backend can deal with it whenever that might end up being (which is why I kept a chunk of the old code in the comments).

Sorry, I just tagged you for review somehow, just undone it.

@mykdavies mykdavies removed the request for review from zachatrocity July 13, 2023 15:40
@shocklateboy92
Copy link
Collaborator

So, you mentioned "to see how people really use it" but we don't collect telemetry. With the exception of the fixed height cards change which really annoyed everybody, we won't find out much about usage/preferences.

If this is a feature you really want to use, you can make a simple implementation. But I agree with @zachatrocity that this should be done in the backend. Rust code isn't much harder than any other code - you should make a pull request to lemmy.

@mykdavies
Copy link
Contributor Author

Oh I would love it to be fixed in the backend, but I'm not jumping into that pit right now.

There have been a number of requests and discussions in /c/liftoff for the user to be able to block instances, and I am strongly influenced by the particular concern about the really NSFW material from lemmynsfw being impossible to block in any meaningful way without ending up cutting off the "you can see a lady's belly" use of the tag in mainstream communities like /c/memes and /c/AccidentalRenaissance. I certainly find browsing all@lemmy.world can be tedious whenever the lemmynsfw posters are busy, but would not want to block all NSFW-tagged posts.

Sure it looks like a lot of code, but mostly that's about shuffling the Settings page about. I think the change to the core home_tab.dart code is as minimal as it can be, but I'm happy to take advice on this.

We would definitely find out how well this works as a solution from feedback within the community, as they've been able to direct our work on a number of occasions already through praise, criticism and suggestions. If it is good enough we'll have a win, but if it's not, we can revert to existing behaviour and ask our users to wait for the Lemmy devs to give it their attention.

I know it appears that a lot of our users want more and better access to porn, but I think a great many would appreciate more control over what turns up in their timeline.

I'm not going to fight this to the death and wouldn't want to have code in the base which isn't welcome, but I do think there's a strong case for this to go ahead.

Happy to hear your responses on this.

@jordan1776
Copy link

To me this is a useful feature. Although on the UI side, a space separated list wouldn't be very intuitive to a casual user. I would go with a list right on the page, with an x next to each item to remove a filter. And a button above to add to the list. Tapping on the text side of the list for editing an item. This would be more mobile friendly.

@shocklateboy92
Copy link
Collaborator

and I am strongly influenced by the particular concern about

Okay, it's starting to sound like a feature you would use personally 😄 .
In that case, I agree with @jordan1776 's point about space separated instances. Large blocks of text config is difficult on mobile for power users, let alone "the normies".

Also, I really think this should be account specific. I.e. if I block lemmynsfw.com from my regular account for browsing during the day, I would very much like to be able to get it back when the mood strikes me.

@jordan1776
Copy link

Also, I really think this should be account specific

I would agree with this. It might be a good idea to add it into the "blocks" page and rename it "Blocks and Filters." Although maybe mixing account and app settings might be confusing so just making it its own settings tab would be better.

@mykdavies
Copy link
Contributor Author

I want to keep this to the minimal thing that could work because it's possible that we will end up ripping this all out once the backend guys do something. I also don't want the people who do want this putting a lot of energy into blocking an ever-increasing list of specific instances when something as simple as "nsfw porn yiff" would immediately cover 99% of the issue. If that solution is good enough for those users who are asking for this then great, if they (and others) start asking for more features, then we know there's enough interest to start working up a more fully-featured solution.

@zachatrocity
Copy link
Collaborator

I think with really minimal code we could make this more straight forward for users. Could we add a submit for each tag and add them as chips with the X's to remove them?

The space separation isn't super intuitive for new users. Also if the backend added this capability the UI/widget will be reused anyways so it wouldn't be wasted effort. Let's add that and then I'm good with this

@shocklateboy92
Copy link
Collaborator

Actually, can we just add a button to the context menu saying "Block instance"?
That way, it'll sit right next to the "Block user" and "Block community" buttons. I feel like that would be the most intuitive.

@zachatrocity
Copy link
Collaborator

@shocklateboy92 that doesn't fit the feature request really. You might be on lemmy.world and wanna block all yiff content regardless of community but you wouldn't wanna block you're instance

@shocklateboy92
Copy link
Collaborator

shocklateboy92 commented Jul 19, 2023

I assume you'd realize you want to block all of yiff because you saw a post from a yiff instance on your feed. You can then choose to block that instance right then/there and never see anything from them again.

And to clarify: if you see something on !blah@yiff.com served via lemmy.world and you click "Block instance", you'd block yiff.com and not lemmy.world.

@mykdavies
Copy link
Contributor Author

I think with really minimal code we could make this more straight forward for users. Could we add a submit for each tag and add them as chips with the X's to remove them?

Okay, that sounds a good approach. I'll give it a go.

@zachatrocity
Copy link
Collaborator

I assume you'd realize you want to block all of yiff because you saw a post from a yiff instance on your feed. You can then choose to block that instance right then/there and never see anything from them again.

And to clarify: if you see something on !blah@yiff.com served via lemmy.world and you click "Block instance", you'd block yiff.com and not lemmy.world.

But what about new communities related to yiff that pop up? Blocking the instance isn't gonna work. Also that might bring unintended consequences due to federation.

@mykdavies
Copy link
Contributor Author

mykdavies commented Jul 20, 2023

Screenshot:
Screenshot 2023-07-20 at 22 41 41

(I'll have a look at the merge conflicts tomorrow)

@mykdavies
Copy link
Contributor Author

Commit c2f5a9b was the result of me merging master into my branch to resolve conflicts, which seems to have ultimately done the right thing but in the most cumbersome way. Hopefully this is all still reviewable...

@@ -82,6 +82,10 @@ abstract class _ConfigStore with Store {
@JsonKey(defaultValue: true)
bool blurNsfw = true;

@observable
@JsonKey(defaultValue: <String>[])
List<String> instanceFilter = [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you change this to a Dictionary of account -> to instanceFilter?

@mykdavies
Copy link
Contributor Author

At this point I just want to have the simplest solution that meets the user requirement as expressed. If this drives enough end-user discussion about different approaches then that's the time to look at how to meet those needs.

Copy link
Collaborator

@zachatrocity zachatrocity left a comment

Choose a reason for hiding this comment

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

This is a really good feature and I agree, lets keep it simple.. get it out there and see how people use it.

LGTM

@mykdavies mykdavies merged commit 8d7a447 into master Aug 1, 2023
5 checks passed
@shocklateboy92 shocklateboy92 deleted the 371-all-user-to-block-instances-in-the-app branch August 21, 2023 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow user to block instances in the app
5 participants