-
Notifications
You must be signed in to change notification settings - Fork 20
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
Populate the dedicated view of direct messages ("People") in the rooms list #323
base: main
Are you sure you want to change the base?
Conversation
Screencast.from.2025-01-14.16-12-01.webmI will fix the issue in a later pr that user can select common room and direct room at the same time if you donnot mind. :) |
Hmm, I don't think we want to have separate scrollable lists for people and rooms -- those should be combined into a single scrollable list. So the goal would be to have separate sections/headings within a single scroll list. That design will also work well with the room search/filter function, in which we may want to show only direct people rooms or only regular public rooms. It will also not waste vertical space, like what's shown in your screenshot with all the whitespace between the |
Single list now:Screencast.from.2025-01-15.15-31-38.webm |
Considering that the implementation methods are quite different, I force pushed. |
Bug:Screencast.from.2025-01-15.18-01-48.webm |
// | ||
// And some specially common items: `<Empty>`, their count is equal to `self.not_direct_rooms_count`. | ||
// `count + self.not_direct_rooms_count + 5` is total, we minus one as index. | ||
let list_end_index = count + self.not_direct_rooms_count + 4; |
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 don't think you need to increase the length of portal list for the headers and draw the header as an item of the portal list. You can draw normal text label "People" or "Room" before the drawing the next item of the Portal List
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.
Yeah, right, no need to put an empty <view>
.
issue #139