-
-
Notifications
You must be signed in to change notification settings - Fork 864
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
Add site.content_warning, local_site.default_post_listing_mode #4393
Conversation
5503f76
to
d8aa6b3
Compare
crates/db_views/src/post_view.rs
Outdated
pub struct PostQuery<'a> { | ||
#[builder(!default)] | ||
pub local_site: LocalSite, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to change this to builder to ensure that LocalSite gets passed in. Using Default is not a good idea because LocalSite itself implements Default, so it would be very easy to forget the parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LocalSite can instead be added to the list function's parameters, which is how non-optional PersonId is done for other views if i remember correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Youre right, I could figure this out but now I got it. Wasted a lot of time rewriting all the code and tests to use builder.
Content warning should be in Site instead of LocalSite so it can be federated in the future. This would allow it to be included in PostView and CommmunityView. Clients can use this to show the content warning before showing an image or community. |
2bcef2d
to
52225e6
Compare
@dullbananas Makes sense, changed it and made it federate. |
a53bf2c
to
87954f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 comment, but other than that it looks good. Feel free to merge whenever.
@@ -39,6 +39,8 @@ pub struct Instance { | |||
pub(crate) image: Option<ImageObject>, | |||
#[serde(default)] | |||
pub(crate) language: Vec<LanguageTag>, | |||
/// nonstandard field | |||
pub(crate) content_warning: Option<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to look for a standard compliant way to federate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saldy I cant find anything as we are already using content and summary. Have to leave it like this.
Looks like we forgot about this. Its good to merge right? |
Yep seems fine. |
I noticed that lemmynsfw runs a fork with some reasonable changes, and decided to integrate them into Lemmy. Specifically there is a new optional value
LocalSite.content_warning
. If it is present, nsfw posts/communities are shown by default. Also frontends and apps should display the content warning when the instance is opened for the first time, and nsfw images shouldnt be blurred. There is also a community settingonly_followers_can_vote
with relevant adjustments in lemmy-ui to disable vote buttons.You can see the forked code under these links (switch to "Files changed" tab):
The frontend also has an additional changes to expand images by default, we could add a backend setting for this too.