Replace MatrixClient.isRoomEncrypted by MatrixClient.CryptoApi.isEncryptionEnabledInRoom in FilePanel
#28275
Conversation
…ncryptionEnabledInRoom` in FilePanel
| @@ -265,7 +270,7 @@ class FilePanel extends React.Component<IProps, IState> { | |||
| /> | |||
| ); | |||
|
|
|||
| const isRoomEncrypted = this.noRoom ? false : MatrixClientPeg.safeGet().isRoomEncrypted(this.props.roomId); | |||
| const isRoomEncrypted = this.noRoom ? false : this.state.isRoomEncrypted; | |||
There was a problem hiding this comment.
This seems to have a risk of being racy, would we not need an interstitial loading state while we figure out if the room is encrypted? Otherwise the search warning may flicker in
| if (client === null) return; | ||
|
|
||
| if (!client.isRoomEncrypted(this.props.roomId)) return; | ||
| if (client === null || !this.state.isRoomEncrypted) return; | ||
|
|
||
| if (EventIndexPeg.get() !== null) { | ||
| client.removeListener(RoomEvent.Timeline, this.onRoomTimeline); |
There was a problem hiding this comment.
Since componentDidMount is async, I think there's another potential for races here: the component mounts, then if it unmounts before componentDidMount even finishes determining whether the room is encrypted, it will set up event listeners that will never get removed.
| @@ -173,7 +181,7 @@ class FilePanel extends React.Component<IProps, IState> { | |||
| // the event index to fulfill the pagination request. Asking the server | |||
| // to paginate won't ever work since the server can't correctly filter | |||
| // out events containing URLs | |||
| if (room && client.isRoomEncrypted(roomId) && eventIndex !== null) { | |||
| if (room && this.state.isRoomEncrypted && eventIndex !== null) { | |||
| return eventIndex.paginateTimelineWindow(room, timelineWindow, direction, limit); | |||
| } else { | |||
| return timelineWindow.paginate(direction, limit); | |||
There was a problem hiding this comment.
Will anything go wrong if this branch gets executed while this.state.isRoomEncrypted is null?
There was a problem hiding this comment.
I conditioned the TimelinePanel under the isRoomEncryptedLoaded check. So this case should not happen now
| if (client.isRoomEncrypted(roomId) && eventIndex !== null) { | ||
| if (this.state.isRoomEncrypted && eventIndex !== null) { |
There was a problem hiding this comment.
Same kind of question here… will this method only get called after this.state.isRoomEncrypted is loaded? If not, is that okay?
|
Putting back to draft, I have to add extra tests. Since I have to take time for this component, I will refactor the component into a functional component |
Checklist
public/exportedsymbols have accurate TSDoc documentation.Task #26922
Replace
MatrixClient.isRoomEncryptedbyMatrixClient.CryptoApi.isEncryptionEnabledInRoomin FilePanel.The new function is async.