fix(mentions): fix wrong user list given to mentions model #16615
Conversation
jrainville
requested review from
micieslak,
caybro and
alexjba
as code owners
|
@caybro you've been randomly selected to test this fix |
| return chatCommunitySectionModule.membersModel | ||
| } | ||
| return usersModule ? usersModule.model : null | ||
| } |
There was a problem hiding this comment.
I moved the logic from ChatView to this store and reused the store whenever we need to get the right member model
Jenkins BuildsClick to see older builds (7)
|
| @@ -175,21 +175,17 @@ StatusSectionLayout { | |||
| rightPanel: Component { | |||
| id: userListComponent | |||
| UserListPanel { | |||
| readonly property var usersStore: ChatStores.UsersStore { | |||
There was a problem hiding this comment.
WDYT of passing the store, just like every other store available here, from the AppMain? Would be nice not to use these context properties here. I know the context properties are available in this component, but I wouldn't extend any impl that relies on this to make it easier for a future refactoring.
There was a problem hiding this comment.
Or even better, forward just the usersModel if it's an option?
There was a problem hiding this comment.
I'm not sure I understand your suggestion. The usersModule is for this chat in particular. We cannot use UsersStore in AppMain, because it wouldn't have access to chatContentModule
There was a problem hiding this comment.
Got it! The suggestion was not to use the context property in the ChatView.
| property var usersModule | ||
|
|
||
| readonly property var usersModel: usersModule ? usersModule.model : null | ||
| readonly property var usersModel: { |
There was a problem hiding this comment.
The usersModule.model provides all contacts and the chatCommunitySectionModule.membersModel provides a list of community members?
There was a problem hiding this comment.
Pretty much. This is a change I did lately where I put a User's module in the section module itself, to hold all the members of the community.
The reason for that being all the public channels of a community contain all the members of a community and so we were duplicating the same member model in all of them, and they were big.
So when a community channel is public (no token permissions), we instead use the section's member model. It saves memory and also improves performance when switching from one public channel to another, because the model doesn't change.
jrainville
force-pushed
the
fix/no-suggested-mentions
branch
from
8fd4477 to
4e0d6e9
Compare
Fixes #16602 This was broken when we refactored the members to use a single model for public channels. Those public channels then didn't have any members in their model they used for suggestions. This is fixed by putting the logic in the UsersStore and reusing that store whenever we need a list of the members.
jrainville
force-pushed
the
fix/no-suggested-mentions
branch
from
4e0d6e9 to
57a5a66
Compare
Fixes #16602 This was broken when we refactored the members to use a single model for public channels. Those public channels then didn't have any members in their model they used for suggestions. This is fixed by putting the logic in the UsersStore and reusing that store whenever we need a list of the members.
What does the PR do
Fixes #16602
This was broken when we refactored the members to use a single model for public channels. Those public channels then didn't have any members in their model they used for suggestions. This is fixed by putting the logic in the UsersStore and reusing that store whenever we need a list of the members.
Affected areas
ChatColumnView and ChatView
Architecture compliance
My PR is consistent with this document: Status Desktop Architecture Guide
Screenshot of functionality (including design for comparison)
mentions.webm
I forgot to show in the video, but they work in the Edit message input too.
Impact on end user
Fixes the mentions
How to test
Test the mentions in different types of chats and scenarios (1-1, group, community channel (public or not), edit message.
Risk
Tick one:
Worst case the mentions are still missing in some context