-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Use context provided RoomViewStore within the RoomView component hierarchy #31077
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
Conversation
Update ContentMessages.ts
| member instanceof RoomMember && member.roomId | ||
| ? member.roomId | ||
| : SdkContextClass.instance.roomViewStore.getRoomId(); | ||
| const memberOrRoomRoomId = member instanceof RoomMember && member.roomId ? member.roomId : 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.
I think this identifier is confusing, if anything it should be memberRoomIdOrRoomRoomId but that isn't really better, what was wrong with roomId? Just the clash? Then maybe the receiver should be tweaked to avoid this confusion. Or you could mutate it if (member instanceof RoomMember && member.roomId) roomId = member.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.
Yea it was mainly the clash.
What do you mean by "Then maybe the receiver should be tweaked to avoid this confusion."?
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.
The parameter name could be fallbackRoomId or something
…nt-web into langleyd/use-context-rvs
depends on #30980
What's in this PR?
This PR builds upon #30980 updating usages of RoomViewStore within the RoomView sub components to use the instance provided by the context rather than the singleton.
Easiest read commit by commit.