-
-
Notifications
You must be signed in to change notification settings - Fork 828
Add options to pin unread/mentioned rooms to the top of the room list #1936
Add options to pin unread/mentioned rooms to the top of the room list #1936
Conversation
Fixes element-hq/element-web#6604 Fixes element-hq/element-web#732 Fixes element-hq/element-web#1104 Signed-off-by: Travis Ralston <travpc@gmail.com>
6bd6add
to
52cdf60
Compare
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.
@lukebarnard1 you might want to take a look at this is your room list store stuff. From my PoV
- I dislike that this logic for room ordering is now split between here (the component) and RoomListStore.
- Surely a sort function would make more sense here, rather than making separate lists of rooms with unread / notifications, de-duplicating them then concatenating them?
Edit: have also commented on element-hq/element-web#732
Originally there was a sort in there, however the sort gets complicated due to the tri-state nature: mentioned, notified (unread), and unread. Having the fused list makes it a little easier to understand in my opinion. |
Two stores updating the same view isn't the worst thing in the world, I don't think. In this case the settings store isn't fluxy but the model still roughly holds. Having said that, the RoomListStore already imports a lot of stuff and there's a |
@turt2live |
@bwindels thanks. Should be good now |
Fixes element-hq/element-web#6604
Fixes element-hq/element-web#732
Fixes element-hq/element-web#1104
Fixes element-hq/element-web#7319