Commit db3fe201 authored by robotjini's avatar robotjini

Fixed three issues related to chatroom loading speed:

1.Changed the loading of moderator roles until after the room loads up so it does not block the loading of the ui. There isn't actually anything that uses the moderator roles right now, so i am leaving it to the implementer of that on how to best handle the delayed information. I wanted to make sure the functionality was the same after the changes though rather than just deleting it.

2. recycler_view was adding its adapter in showMessages this was causing the error: "E/RecyclerView: No adapter attached; skipping layout"
This seems to be the cause of a missed update step and ui delay. Code to add adapter was added to setupRecyclerView, which is called in the onViewCreated function.

3. loadActiveMembers function was being called from ChatRoomFragment object and the network / data access was being done in the UI thread. Moved the function call from the end of the function loadMessages to just after the end of said function. Changed the function loadActiveMembers to do most of its work in an I.O. thread and just the view updating in a UI thread.
parent 5ceacaf2
...@@ -117,6 +117,7 @@ class ChatRoomPresenter @Inject constructor( ...@@ -117,6 +117,7 @@ class ChatRoomPresenter @Inject constructor(
private var chatRoomId: String? = null private var chatRoomId: String? = null
private lateinit var chatRoomType: String private lateinit var chatRoomType: String
private lateinit var chatRoomName: String
private var chatIsBroadcast: Boolean = false private var chatIsBroadcast: Boolean = false
private var chatRoles = emptyList<ChatRoomRole>() private var chatRoles = emptyList<ChatRoomRole>()
private val stateChannel = Channel<State>() private val stateChannel = Channel<State>()
...@@ -135,47 +136,73 @@ class ChatRoomPresenter @Inject constructor( ...@@ -135,47 +136,73 @@ class ChatRoomPresenter @Inject constructor(
draftKey = "${currentServer}_${LocalRepository.DRAFT_KEY}$roomId" draftKey = "${currentServer}_${LocalRepository.DRAFT_KEY}$roomId"
chatRoomId = roomId chatRoomId = roomId
chatRoomType = roomType chatRoomType = roomType
chatRoomName = roomName
chatRoles = emptyList()
var canModerate = isOwnerOrMod()
GlobalScope.launch(Dispatchers.IO + strategy.jobs) { GlobalScope.launch(Dispatchers.IO + strategy.jobs) {
try { // Can post anyway if has the 'post-readonly' permission on server.
chatRoles = if (roomTypeOf(roomType) !is RoomType.DirectMessage) { val room = dbManager.getRoom(roomId)
client.chatRoomRoles(roomType = roomTypeOf(roomType), roomName = roomName) room?.let {
} else { chatIsBroadcast = it.chatRoom.broadcast ?: false
emptyList() val roomUiModel = roomMapper.map(it, true)
} launchUI(strategy) {
} catch (ex: Exception) { view.onRoomUpdated(roomUiModel = roomUiModel.copy(
Timber.e(ex)
chatRoles = emptyList()
} finally {
// User has at least an 'owner' or 'moderator' role.
val canModerate = isOwnerOrMod()
// Can post anyway if has the 'post-readonly' permission on server.
val room = dbManager.getRoom(roomId)
room?.let {
chatIsBroadcast = it.chatRoom.broadcast ?: false
val roomUiModel = roomMapper.map(it, true)
launchUI(strategy) {
view.onRoomUpdated(roomUiModel = roomUiModel.copy(
broadcast = chatIsBroadcast, broadcast = chatIsBroadcast,
canModerate = canModerate, canModerate = canModerate,
writable = roomUiModel.writable || canModerate writable = roomUiModel.writable || canModerate
)) ))
}
} }
}
loadMessages(roomId, roomType, clearDataSet = true) loadMessages(roomId, chatRoomType, clearDataSet = true)
chatRoomMessage?.let { messageHelper.messageIdFromPermalink(it) } loadActiveMembers(roomId, chatRoomType, filterSelfOut = true)
chatRoomMessage?.let { messageHelper.messageIdFromPermalink(it) }
?.let { messageId -> ?.let { messageId ->
val name = messageHelper.roomNameFromPermalink(chatRoomMessage) val name = messageHelper.roomNameFromPermalink(chatRoomMessage)
citeMessage( citeMessage(
name!!, name!!,
messageHelper.roomTypeFromPermalink(chatRoomMessage)!!, messageHelper.roomTypeFromPermalink(chatRoomMessage)!!,
messageId, messageId,
true true
) )
} }
subscribeRoomChanges()
/*FIXME: Get chat role can cause unresponsive problems especially on slower connections
We are updating the room again after the first step so that initial messages
get loaded in and the system appears more responsive. Something should be
done to either fix the load in speed of moderator roles or store the
information locally*/
if (getChatRole()) {
canModerate = isOwnerOrMod()
if (canModerate) {
//FIXME: add this in when moderator page is actually created
//view.updateModeration()
}
}
subscribeRoomChanges()
}
}
private suspend fun getChatRole() : Boolean {
var returnVal = false
try {
if (roomTypeOf(chatRoomType) !is RoomType.DirectMessage) {
chatRoles = withContext(Dispatchers.IO + strategy.jobs) {
client.chatRoomRoles(roomType = roomTypeOf(chatRoomType), roomName = chatRoomName)
}
returnVal = true
} else {
chatRoles = emptyList()
} }
} catch (ex: Exception) {
Timber.e(ex)
chatRoles = emptyList()
} }
return returnVal
} }
private suspend fun subscribeRoomChanges() { private suspend fun subscribeRoomChanges() {
...@@ -209,8 +236,8 @@ class ChatRoomPresenter @Inject constructor( ...@@ -209,8 +236,8 @@ class ChatRoomPresenter @Inject constructor(
) { ) {
this.chatRoomId = chatRoomId this.chatRoomId = chatRoomId
this.chatRoomType = chatRoomType this.chatRoomType = chatRoomType
launchUI(strategy) {
view.showLoading() GlobalScope.launch(Dispatchers.IO + strategy.jobs) {
try { try {
if (offset == 0L) { if (offset == 0L) {
// FIXME - load just 50 messages from DB to speed up. We will reload from Network after that // FIXME - load just 50 messages from DB to speed up. We will reload from Network after that
...@@ -246,8 +273,6 @@ class ChatRoomPresenter @Inject constructor( ...@@ -246,8 +273,6 @@ class ChatRoomPresenter @Inject constructor(
}.ifNull { }.ifNull {
view.showGenericErrorMessage() view.showGenericErrorMessage()
} }
} finally {
view.hideLoading()
} }
subscribeTypingStatus() subscribeTypingStatus()
...@@ -793,13 +818,15 @@ class ChatRoomPresenter @Inject constructor( ...@@ -793,13 +818,15 @@ class ChatRoomPresenter @Inject constructor(
} }
} }
fun loadActiveMembers( suspend fun loadActiveMembers(
chatRoomId: String, chatRoomId: String,
chatRoomType: String, chatRoomType: String,
offset: Long = 0, offset: Long = 0,
filterSelfOut: Boolean = false filterSelfOut: Boolean = false
) { ) {
launchUI(strategy) { val activeUsers = mutableListOf<PeopleSuggestionUiModel>()
withContext(Dispatchers.IO + strategy.jobs) {
try { try {
val members = retryIO("getMembers($chatRoomId, $chatRoomType, $offset)") { val members = retryIO("getMembers($chatRoomId, $chatRoomType, $offset)") {
client.getMembers(chatRoomId, roomTypeOf(chatRoomType), offset, 50).result client.getMembers(chatRoomId, roomTypeOf(chatRoomType), offset, 50).result
...@@ -810,7 +837,6 @@ class ChatRoomPresenter @Inject constructor( ...@@ -810,7 +837,6 @@ class ChatRoomPresenter @Inject constructor(
// Take at most the 100 most recent messages distinguished by user. Can return less. // Take at most the 100 most recent messages distinguished by user. Can return less.
val recentMessages = messagesRepository.getRecentMessages(chatRoomId, 100) val recentMessages = messagesRepository.getRecentMessages(chatRoomId, 100)
.filterNot { filterSelfOut && it.sender?.username == self } .filterNot { filterSelfOut && it.sender?.username == self }
val activeUsers = mutableListOf<PeopleSuggestionUiModel>()
recentMessages.forEach { recentMessages.forEach {
val sender = it.sender val sender = it.sender
val username = sender?.username ?: "" val username = sender?.username ?: ""
...@@ -849,11 +875,13 @@ class ChatRoomPresenter @Inject constructor( ...@@ -849,11 +875,13 @@ class ChatRoomPresenter @Inject constructor(
) )
}) })
view.populatePeopleSuggestions(activeUsers)
} catch (e: RocketChatException) { } catch (e: RocketChatException) {
Timber.e(e) Timber.e(e)
} }
} }
launchUI(strategy) {
view.populatePeopleSuggestions(activeUsers)
}
} }
fun spotlight(query: String, @AutoCompleteType type: Int, filterSelfOut: Boolean = false) { fun spotlight(query: String, @AutoCompleteType type: Int, filterSelfOut: Boolean = false) {
......
...@@ -425,25 +425,12 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR ...@@ -425,25 +425,12 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR
} }
} }
if (recycler_view.adapter == null) {
recycler_view.adapter = adapter
if (dataSet.size >= 30) {
recycler_view.addOnScrollListener(endlessRecyclerViewScrollListener)
}
recycler_view.addOnLayoutChangeListener(layoutChangeListener)
recycler_view.addOnScrollListener(onScrollListener)
// Load just once, on the first page...
presenter.loadActiveMembers(chatRoomId, chatRoomType, filterSelfOut = true)
}
val oldMessagesCount = adapter.itemCount val oldMessagesCount = adapter.itemCount
adapter.appendData(dataSet) adapter.appendData(dataSet)
if (oldMessagesCount == 0 && dataSet.isNotEmpty()) { if (oldMessagesCount == 0 && dataSet.isNotEmpty()) {
recycler_view.scrollToPosition(0) recycler_view.scrollToPosition(0)
verticalScrollOffset.set(0) verticalScrollOffset.set(0)
} }
presenter.loadActiveMembers(chatRoomId, chatRoomType, filterSelfOut = true)
empty_chat_view.isVisible = adapter.itemCount == 0 empty_chat_view.isVisible = adapter.itemCount == 0
dismissEmojiKeyboard() dismissEmojiKeyboard()
} }
...@@ -465,10 +452,8 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR ...@@ -465,10 +452,8 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR
setupMessageComposer(roomUiModel) setupMessageComposer(roomUiModel)
isBroadcastChannel = roomUiModel.broadcast isBroadcastChannel = roomUiModel.broadcast
isFavorite = roomUiModel.favorite.orFalse() isFavorite = roomUiModel.favorite.orFalse()
if (isBroadcastChannel && !roomUiModel.canModerate) { disableMenu = (roomUiModel.broadcast && !roomUiModel.canModerate)
disableMenu = true activity?.invalidateOptionsMenu()
activity?.invalidateOptionsMenu()
}
} }
} }
...@@ -803,7 +788,12 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR ...@@ -803,7 +788,12 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR
presenter.loadMessages(chatRoomId, chatRoomType, page * 30L) presenter.loadMessages(chatRoomId, chatRoomType, page * 30L)
} }
} }
recycler_view.adapter = adapter
recycler_view.addOnScrollListener(fabScrollListener) recycler_view.addOnScrollListener(fabScrollListener)
recycler_view.addOnScrollListener(endlessRecyclerViewScrollListener)
recycler_view.addOnLayoutChangeListener(layoutChangeListener)
recycler_view.addOnScrollListener(onScrollListener)
} }
private fun setupFab() { private fun setupFab() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment