Skip to content

Mark all inbox messages as read when leaving the inbox #1210

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

folkemat
Copy link
Contributor

I found it annoying to constantly have to press ‘Mark all as read’ in the inbox menu. So this PR adds the option to automatically mark all messages as read when you leave the inbox, toggled via a checkbox in the inbox menu.

@QuantumBadger
Copy link
Owner

Thanks for this! In general it looks fine, although there are potentially a couple of issues (that also exist with the current "mark all as read" option):

  • If the user receives more messages after loading the page (but before going back), those messages will be marked as read despite never being seen
  • Messages which are lower down in the list which weren't seen will also be marked as read

I'm wondering if it might be better to send an /api/read_message request containing only the messages which were actually shown on the user's screen (i.e. when the user scrolls we update a Set, and then when going back we send the request). We could also update the existing menu item to only mark the viewed items as read.

Would be interested to hear your thoughts!

@folkemat
Copy link
Contributor Author

I agree with you, these are valid points. I have implemented the API/read_message function in #1238 for individual messages that are tapped on, it's probably best to merge that PR first

@QuantumBadger
Copy link
Owner

@folkemat Thank you, I'll take a look at that PR.

As a sidenote -- this PR marks all the messages as read regardless of the setting :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants