Skip to content

Make read status on Note to Self only show if read receipts enabled in settings #7241

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 1 commit into
base: main
Choose a base branch
from

Conversation

lamemakes
Copy link
Contributor

Contributor checklist:

  • My contribution is not related to translations.
  • My commits are in nice logical chunks with good commit messages
  • My changes are rebased on the latest main branch
  • A pnpm run ready run passes successfully (more about tests here)
  • My changes are ready to be shipped to users

Description

A pretty minute change to make the desktop client consistent with Android client (didn't have the resources to test iOS at time of writing). Currently, even if read receipts are off, a Note to Self message will show a status icon of read once successfully sent. This change makes Notes to Self messages only show as delivered when read receipts are disabled.

Along with mirroring the Android Signal client's functionality, this makes more sense to me personally as it could be confusing to users to have read receipts showing up at all after disabling.

Thanks for taking a look, Cheers!

@lamemakes
Copy link
Contributor Author

while digging in the code it seemed the easiest way to access the read receipt setting was via the read-receipt-setting storage item, happy to swap that for another wrapper or something if there's a better way!

@scottnonnenberg-signal
Copy link
Contributor

Thanks for looking at this - before I look at the code, I'll be talking to our design folks, to see if this change is what we want.

@lamemakes
Copy link
Contributor Author

thanks @scottnonnenberg-signal! happy to raise an issue in the android client repo if this is decided against

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