-
-
Notifications
You must be signed in to change notification settings - Fork 832
Always insert rooms into lists when they get lost #2736
Conversation
Room upgrades, direct chats, etc all end up being lost in these scenarios. Instead of losing them to the list, try and put them into a relevant spot of the list. Fixes element-hq/element-web#9020
src/stores/RoomListStore.js
Outdated
} | ||
} else { | ||
insertedIntoTags.push(key); | ||
// There's some circumstances where the room doesn't fit anywhere, so just |
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 it would be good to detail here what the circumstance are, if we know them now.
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.
It's almost always "ran off the list" - I've adjusted the approach to instead try and fix this more calmly.
insertedIntoTags.push(key); | ||
} else { | ||
// In theory, this should never happen | ||
console.warn(`!! Room ${room.roomId} lost: No position available`); |
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.
Is it worth keeping some kind of logging that it would have been lost without this new fallback? (Do we hope to do something better in the future?)
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.
Absolutely - have added a bunch of logging
Instead of having a catch-all insert, try and fix the common cases with a bit more care.
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.
Thanks, hopefully we'll be able to keep improving this with the extra info.
Room upgrades, direct chats, etc all end up being lost in these scenarios. Instead of losing them to the list, try and put them into a relevant spot of the list.
Fixes element-hq/element-web#9020
Demo:
Repeating the steps without this patch results in rooms disappearing from the room list.