-
Notifications
You must be signed in to change notification settings - Fork 342
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
Feature/mark post as read button #1135
Feature/mark post as read button #1135
Conversation
* I18 quality of life change * Cleanup
* Add logged out messages to profile and community * Remove errors when not logged in * Add logged out translations
* Add content warning to modlog and fix modlog routing bug * Add translation logic
* Show language on posts and comments * Revert "Show language on posts and comments" This reverts commit 5426790. * Change language indicator look
It doesnt need to be emphasized so much that a user is banned. Anyway this can already be seen in the mod log. For users who are banned from the entire site it is still shown on the profile. Co-authored-by: Dessalines <[email protected]>
Lemmy-ui currently preselects the first language in the user settings when creating a new post or comment. This is a bad idea because this language might not actually be allowed in the community. It is better to pass the language as None if the user didnt specify it explicitly, because then the backend can smartly choose a language based on the overlap of user languages and community languages. This fixes the problem described in [this thread](https://lemmy.ml/post/1066608), where a user tries to post in a community that has only English allowed, with all languages enabled in user settings. In this case lemmy-ui preselects "undetermined language" as default, which is not allowed and results in an error. This PR fixes the issue because it lets the backend automatically select the correct language (English).
Changes to language tag
* Fix isoData can contain user generated content * Fix formatting
There are a bunch of changes from old commits showing up in this PR. |
This is because I had to use branch release/0.17 while developing since the UI on main and BE on main and/or BE on 0.17 and UI on main aren’t compatible. |
I'll try pulling this branch and merging main into it since the latest changes work on my machine. |
@ItsEcholot I did a merge. Can you allow me to push changes to this branch? |
@SleeplessOne1917 Sure, sent you an invite to the fork. |
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.
Besides the other comments I left, I'm gonna hold off on thinking about merging this until the websocket API removal has hit main.
@@ -750,6 +752,25 @@ export class PostListing extends Component<PostListingProps, PostListingState> { | |||
); | |||
} | |||
|
|||
get markReadButton() { | |||
const read = this.props.post_view.read; | |||
const label = read ? i18n.t("mark read") : i18n.t("mark unread"); |
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.
Please don't put spaces in the translation strings. Use mark_read
and mark_unread
instead.
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.
These are the types of features that should be waiting till after our HTTP rework. I'd just put this on hold until then, and then rebase it. Shouldn't be more than a few days.
@ItsEcholot there are some merge conflicts that need handling before this can be merged. |
I did some basic merging but didn't have time to test the functionality atm... If someone else wants to do some testing and fixing (CI errors as well) it would be appreciated, else I'm gonna get around to it myself some time in the future. |
Closing as this is stale and needs a rebase. |
Fixes #384