-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Second batch: Replace MatrixClient.isRoomEncrypted
by MatrixClient.CryptoApi.isEncryptionEnabledInRoom
#28466
base: develop
Are you sure you want to change the base?
Second batch: Replace MatrixClient.isRoomEncrypted
by MatrixClient.CryptoApi.isEncryptionEnabledInRoom
#28466
Conversation
…ncryptionEnabledInRoom` in `MemberListStore.tsx`
…ncryptionEnabledInRoom` in `EventIndex.tsx`
…ncryptionEnabledInRoom` in `SendMessageComposer.tsx`
…ncryptionEnabledInRoom` in `ScalarMessaging.ts`
…ncryptionEnabledInRoom` in `RolesRoomSettingsTab.tsx`
* Async version of Array.filter. | ||
* @param values | ||
* @param predicate |
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.
This doc could do with clarification on what happens when a rejection occurs
@@ -841,7 +841,7 @@ async function readEvents( | |||
} | |||
} | |||
|
|||
const onMessage = function (event: MessageEvent<any>): void { | |||
const onMessage = async function (event: MessageEvent<any>): Promise<void> { |
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 we sure this method can be async? It could make events be handled out-of-order unless we add something to serialise the data flow
// hide the power level selector for enabling E2EE if it the room is already encrypted | ||
if (client.isRoomEncrypted(this.props.room.roomId)) { | ||
if (this.state.isRoomEncrypted) { | ||
delete eventsLevels[EventType.RoomEncryption]; | ||
} |
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.
This looks like it'll cause a UI flicker, where a row disappears after all rooms are checked, this is undesirable, we should not show UI until we are sure it is relevant.
@@ -70,7 +70,7 @@ export class MemberListStore { | |||
return []; | |||
} | |||
|
|||
if (!this.isLazyLoadingEnabled(roomId) || this.loadedRooms.has(roomId)) { | |||
if (!(await this.isLazyLoadingEnabled(roomId)) || this.loadedRooms.has(roomId)) { |
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.
Can we reverse this if condition so that the synchronous path is not blocked behind the async one?
if (await this.props.mxClient.getCrypto()?.isEncryptionEnabledInRoom(this.props.room.roomId)) { | ||
this.prepareToEncrypt = throttle( | ||
() => { | ||
this.props.mxClient.getCrypto()?.prepareToEncrypt(this.props.room); | ||
}, | ||
60000, | ||
{ leading: true, trailing: false }, | ||
); | ||
} |
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.
What happens if isEncryptionEnabledInRoom
is still resolving by the time the user types something, encryption won't be prepared it looks like, this is undesirable and a regression
Checklist
public
/exported
symbols have accurate TSDoc documentation.Task #26922
Replace
MatrixClient.isRoomEncrypted
byMatrixClient.CryptoApi.isEncryptionEnabledInRoom