-
-
Notifications
You must be signed in to change notification settings - Fork 827
Use Sets instead of array scans and simplify hiding of invalid users when inviting #4004
Conversation
…to invite Signed-off-by: Michael Telatynski <[email protected]>
Is there a bug here or is this just cleanup? |
As the first post says: |
@@ -456,7 +453,7 @@ export default class InviteDialog extends React.PureComponent { | |||
const events = room.getLiveTimeline().getEvents(); // timelines are most recent last | |||
for (let i = events.length - 1; i >= Math.max(0, events.length - maxMessagesConsidered); i--) { | |||
const ev = events[i]; | |||
if (excludedUserIds.includes(ev.getSender())) { | |||
if (excludedTargetIds.has(ev.getSender())) { |
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 was not skipping on the members already in the room
I honestly don't see "Fixes XXXX" unless it is bold, sorry :( |
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.
Not sure I see the benefit of sets, but this seems to work.
Signed-off-by: Michael Telatynski <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
Set has closer to |
sure, but it's still just as fast. It's too small of a dataset to really make much of a difference. |
ah its limited at 200, fair enough |
The difference between
excludedTargetIds
andexcludedUserIds
may be lost on me :(it was pruning out the
excludedTargetIds
then adding anylastSpoke
users back in anyway to suggestionsSeeing so many nested loops and
.includes
calls hurt me deeply so I flipped things over to use ES6 Sets.Fixes element-hq/element-web#12051
Signed-off-by: Michael Telatynski [email protected]