-
Notifications
You must be signed in to change notification settings - Fork 985
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
Offload chat messages #9995
Offload chat messages #9995
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (15)
|
77663f3
to
753eca6
Compare
ebb97ed
to
388f898
Compare
99% of end-end tests have passed
Failed tests (1)Click to expand
Passed tests (96)Click to expand
|
@cammellos The same is on navigating to chat - redirect happens faster, though there is a delay on message appearing (not noticeable actually so fine) This item is not really clear to me:
|
@Serhy thanks for checking
Hope is clear, thanks for taking the time! |
Aha, I've got it, @cammellos ! |
388f898
to
62654db
Compare
@cammellos tested on Galaxy S8+, loaded both |
@@ -334,6 +344,7 @@ | |||
:idx idx | |||
:list-ref messages-list-ref}]) | |||
:inverted true | |||
:onViewableItemsChanged on-viewable-items-changed |
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.
are the default good enough?
there is a bunch of visibility config for this for instance
:viewability-config (clj->js {:waitForInteraction false
:minimumViewTime 100
:viewAreaCoveragePercentThreshold 60})
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.
They should be ok
@@ -0,0 +1,3 @@ | |||
(ns status-im.ui.screens.chat.state) | |||
|
|||
(defonce viewable-item (atom nil)) |
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.
shouldn't this be cleaned after logout?
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.
I can probably clean it up as we land on a chat, it's probably safer, but should get reset as soon as we load messages
@errorists that should be enough, thanks! |
acc)) | ||
{} | ||
(get-in db [:chats chat-id :messages]))] | ||
(fx/merge cofx |
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.
fx/merge
and -> db
looks redundant here
a22f85e
to
4734a4e
Compare
This commit does a few things: 1) Messages are offloaded from any chat once we go back from the home. This allows us to ignore any message that is coming in from a chat we are not currently focused. 2) After 5 seconds of not-scrolling activity, any received message that is not currently visible will be offloaded to the database. 3) Similarly received messages that are not visible will be offloaded to the database directly Signed-off-by: Andrea Maria Piana <andrea.maria.piana@gmail.com>
This allows us to ignore any message that is coming in from a chat we
are not currently focused.
is not currently visible will be offloaded to the database.
the database directly
This should improve performance in the following areas:
The drawback is that when you go from home -> to a chat, you won't see the messages immediately (it will always be the same as it is now when you first go to a chat after login, as messages will be loaded from the database), but this is likely necessary (or will be changed) anyway once we implement scroll to last not seen message.
status: ready