Skip to content
This repository has been archived by the owner on Jul 9, 2022. It is now read-only.

Support for markAsDelivered and auto message delivery and reading. #768

Merged
merged 2 commits into from
Dec 18, 2019

Conversation

HossamMohsen7
Copy link
Contributor

@HossamMohsen7 HossamMohsen7 commented Dec 6, 2019

Facebook in the browser marks new messages as delivered, So I added that here with the abillity to make it automatic (On by default). Also made an option to make markAsRead automatic but it's not on by default.

I don't know for sure if marking messages as delivered helps with the issue of accounts getting blocked, but it's better to do it anyway.

I fixed an issue I saw in #763 related to listenMqtt too.

Also added the docs to these new changes.
Thanks!

@Schmavery
Copy link
Owner

@7osCraft thanks a lot, I will review this soon.

@lequanglam you said here that you got blocked less when you mark as read? Should we turn this on by default?

@DanH42
Copy link

DanH42 commented Dec 6, 2019

I use a custom user agent (pulled from my own web browser and updated periodically) but I don't automatically mark all messages as read, and I haven't been flagged in months. Instantly marking every single message as read 24/7 also sounds like bot behavior, so it's not something I'd want to happen by default.

I'll also mention that keeping track of appState made a huge difference in preventing security checkpoints, so I'd encourage anyone who's not using it to do so.

@HossamMohsen7
Copy link
Contributor Author

Yes, auto marking as seen might be bot behaviour so that's why I made it false by default, but marking as delivered is done automatically by the browser or the app even if you leave it. Unless you go offline of course.

It also looks like some values in appState changes over time so I think we should do something about it.

@HossamMohsen7
Copy link
Contributor Author

Another thing I do is sending typing indicators before every message. I might add that to the PR.

@Schmavery
Copy link
Owner

Schmavery commented Dec 6, 2019

@7osCraft if you do the typing indicator change can you make it a separate PR please <3

@HossamMohsen7
Copy link
Contributor Author

Sure 😃

@Schmavery
Copy link
Owner

I agree it would be cool to look into making appState easier to use or at least improving the docs around it so that it's less confusing

Copy link
Owner

@Schmavery Schmavery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good, thanks for including the docs changes as well.
I left just a couple of small comments.

DOCS.md Outdated Show resolved Hide resolved
src/listenMqtt.js Outdated Show resolved Hide resolved
@HossamMohsen7
Copy link
Contributor Author

Pushed some changes and fixes.

Copy link
Owner

@Schmavery Schmavery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great thanks!

@Schmavery Schmavery merged commit 9e949ad into Schmavery:master Dec 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants