-
-
Notifications
You must be signed in to change notification settings - Fork 859
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
On registration set show_nsfw based on site.content_warning #4616
Conversation
07e2c08
to
457789e
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.
Since its required -> optional, it should be non-breaking change.
This is the only reasonable option. Making show_nsfw in the form optional is not useful for that, and it just adds complexity. |
@dullbananas That would mean removing the |
It wouldn't necessarily remove it, just completely ignore it (only when site.content_warning exists). If site.content_warning doesn't exist, show_nsfw is still necessary. Either approach works for me. |
I brought up another related point in my frontend PR for this:
|
I thought that meant the initial checkbox value. I think making show_nsfw optional and removing the checkbox in lemmy-ui is the best solution. |
Also this seems to be a breaking change for crates that depend on the api_common crate |
In that case I would expect bug reports that the show_nsfw param is broken. |
I just made it so enable NSFW is automatically selected when content warning is defined in my UI PR. |
Whats the consensus then? |
If we leave |
Do we merge this or not? |
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.
Ya I think its good.
To make this work properly, lemmy-ui during registration should send
show_nsfw = null
if the user doesnt choose a value manually. Or set the default value for show_nsfw based onsite.content_warning
.