Shortcut fixes #4354
Shortcut fixes #4354
Conversation
|
|
||
| val session = activeSessionHolder.getSafeActiveSession() ?: return Disposables.empty() | ||
| pinCodeStore.addListener(this) | ||
| return session.getRoomSummariesLive( |
There was a problem hiding this comment.
getPagedRoomSummariesLive was used before, which was not correct, only the first 20 rooms was returned
There was a problem hiding this comment.
do we need to know about all the rooms? the shortcut menu only holds a few entries~
There was a problem hiding this comment.
Yes, because we have to check if a room has been left (so not in the list anymore) to disable any related shortcuts pinned to the home
| val deadShortcutIds = ShortcutManagerCompat.getShortcuts(context, ShortcutManagerCompat.FLAG_MATCH_DYNAMIC) | ||
| .map { it.id } | ||
| .filter { !roomIds.contains(it) } | ||
| ShortcutManagerCompat.removeLongLivedShortcuts(context, deadShortcutIds) |
There was a problem hiding this comment.
disableShortcuts is also called now
| } | ||
|
|
||
| @Singleton |
There was a problem hiding this comment.
Add this to be able to add listener to this "object"
| awaitPinCodeCallback<Boolean> { | ||
| PFSecurityManager.getInstance().pinCodeHelper.delete(it) | ||
| } | ||
| return@withContext |
There was a problem hiding this comment.
This line was a bit useless to me (?)
| sortOrder = RoomSortOrder.PRIORITY_AND_ACTIVITY | ||
| ) | ||
| .asObservable() | ||
| .doOnDispose { |
There was a problem hiding this comment.
NIT: it could be easier to follow the listeners flow if they're added and removed within the pipeline
.doOnSubscribe { pinCodeStore.addListener(this) }
.doFinally { pinCodeStore.removeListener(this) } // finally catches error, completes and disposed 
There was a problem hiding this comment.
Please be aware the code will change with the Rx-Flow migration
There was a problem hiding this comment.
OK, I can do this small change.
There was a problem hiding this comment.
(Not so small since doOnDispose -> doFinally) 34e8cf8
| private fun createShortcuts(rooms: List<RoomSummary>) { | ||
| if (hasPinCode) { | ||
| // No shortcut in this case (privacy) | ||
| ShortcutManagerCompat.removeAllDynamicShortcuts(context) |
There was a problem hiding this comment.
to double check my understanding, if there's a pin code set we don't allow shortcuts?
There was a problem hiding this comment.
That's not exactly true.
If a PIN code is set it will still be possible to pin (the term is the same :/) a room from the room profile screen, so inside the app, i.e. once the PIN code has been entered. But the limited list of shortcuts (room) that we can see when long pressing on the app icon will not be visible anymore.
So a PIN code prevent a few rooms (5 on my phone) to be disclosed without having to enter the PIN code.
There was a problem hiding this comment.
ah thanks for explaining!
Fixes #4168
Ensure pinned shortcuts are disabled (it is not possible to remove them) if the room is left. To repro:
The shortcut is now disabled
There were another issue related to paged rooms, but now all the room are taken into account
Change in SDK is adding API to sort rooms, without any API break, but please review carefully 🙏
Fixes #4170 :
Remove shortcut as soon as a PIN code is set, for privacy.
I have one concern about performance, shortcut are computed each time any room summary is updated, it's probable possible to optimise that. I am open to ideas.