Skip to content
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 #139

Open
kevinaboos opened this issue Sep 4, 2024 · 14 comments
Assignees
Labels
good first issue Good for newcomers help wanted Looking for help from anyone!

Comments

@kevinaboos
Copy link
Member

kevinaboos commented Sep 4, 2024

Now that we have a new UI, this should be relatively straightforward. All we need to do is determine whether a room is a direct message, which can be done via multiple different SDK APIs:

Then, those "direct" rooms should be placed under the People heading of the rooms list, and thus they should not appear under the Rooms heading any more.

Note that the collapsible section headers don't actually collapse/expand yet -- that is a known missing feature.

@guofoo guofoo added this to Robrix Aug 15, 2024
@kevinaboos kevinaboos converted this from a draft issue Sep 4, 2024
@kevinaboos kevinaboos changed the title Dedicated view of direct messages (DMs) Populate the dedicated view of direct messages ("People") in the rooms list Sep 4, 2024
@kevinaboos kevinaboos moved this to Ready in Robrix Sep 4, 2024
@kevinaboos kevinaboos added good first issue Good for newcomers help wanted Looking for help from anyone! labels Sep 4, 2024
@aaravlu
Copy link
Contributor

aaravlu commented Sep 21, 2024

I'm confused about a few things.

  1. What is the difference between the is_dm field and the is_direct method?
  2. And it seems that the is_dm field is missing from the latest git repository? Maybe I should try the is_dm filed under ssroom ?
  3. I made a copy of rooms_list.rs under home dir as "people_list.rs", then changed all the names related to Rooms to People, and then in sliding_sync.rs I stripped out the dm rooms, to match what's in people_list.rs, and then in rooms_sidebar.rs, I added the PeopleList (like RoomsList ), which shhould have contained the peeled dm rooms in people_list.rs. But why does it still not work?

https://github.com/Demolemon11/robrix/blob/main/src/sliding_sync.rs
The line numbers of the code related to dm rooms stripping in "sliding_sync.rs" are 996, 1016, 1047 and1120

1

@kevinaboos
Copy link
Member Author

I'm confused about a few things.

  1. What is the difference between the is_dm field and the is_direct method?

I think they're the same, but I'm not sure since I haven't tried it myself.

  1. And it seems that the is_dm field is missing from the latest git repository? Maybe I should try the is_dm filed under ssroom ?

Also not sure. I know the sdk is in the middle of major changes, specifically that the old sliding sync is being removed in favor of the new "simplified" sliding sync.

  1. I made a copy of rooms_list.rs under home dir as "people_list.rs", then changed all the names related to Rooms to People, and then in sliding_sync.rs I stripped out the dm rooms, to match what's in people_list.rs, and then in rooms_sidebar.rs, I added the PeopleList (like RoomsList ), which shhould have contained the peeled dm rooms in people_list.rs. But why does it still not work?

Hard to know for sure without seeing your code; if you have some code, feel free to submit a PR. But in general, we don't want to duplicate the views because we want DMs to be shown in the same Rooms List view, so I don't think that's the best way forward.

I think all you'd need to do is when drawing the rooms list, sort each room under the proper Rooms or People heading based on whether it's a DM room.

@aaravlu
Copy link
Contributor

aaravlu commented Sep 21, 2024

Are you saying that my changing sliding_sync.rs is wrong, and I shouldn't have to determine if it's a DM or not in such a low-level module?

@aaravlu
Copy link
Contributor

aaravlu commented Sep 21, 2024

At the begining, I just didnot want to modify sliding_sync.rs, I know the whole structure of the project would be chaos if I peel DM room in sliding_sync.rs
I am a green hand to Rust, Matrix, and makepad, So I hope you understand me : )

@kevinaboos
Copy link
Member Author

Are you saying that my changing sliding_sync.rs is wrong, and I shouldn't have to determine if it's a DM or not in such a low-level module?

ah, no, I wasn't saying that, sorry. You may need to check whether a room is a DM/direct room within the "backend" (the sliding_sync module) and then include that information in the RoomPreviewInfo that gets sent to the UI main thread.

https://github.com/Demolemon11/robrix/blob/main/src/sliding_sync.rs
The line numbers of the code related to dm rooms stripping in "sliding_sync.rs" are 996, 1016, 1047 and1120

Those don't seem to be the correct line numbers, as they're completely irrelevant to DM stuff. If you want a proper code review, please submit a PR -- git has tools for this purpose. Links to separate code files are not reviewable.

At the begining, I just didnot want to modify sliding_sync.rs, I know the whole structure of the project would be chaos if I peel DM room in sliding_sync.rs
I am a green hand to Rust, Matrix, and makepad, So I hope you understand me : )

yea no problem! don't worry, we're here to help as much as possible.

@kevinaboos
Copy link
Member Author

Also, I'm literally attending the Matrix Conference in Berlin today, and this slide came up in a talk about the future of sliding sync (which involves moving from the sliding sync proxy to the sliding sync native version).
image

It seems to indicate that is_dm() is being removed, so I'd recommend that we don't use that function.

@alanpoon
Copy link
Contributor

alanpoon commented Oct 4, 2024

I am looking at this issue.

@alanpoon
Copy link
Contributor

alanpoon commented Oct 5, 2024

It seems like room_list is inside cached_widget.

rooms_list = <RoomsList> {}

It is not possible to differentiate room_type as property in the roomlist's draw_walk function as there can only be 1 instance: e.g

<CachedWidget>{ room_list = <RoomList>{ room_type: "people" } } <CachedWidget>{ room_list = <RoomList>{ room_type: "room" } } fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { self.room_type --> will always be "people". }
@kevinaboos Is there a way to pass "room_type" property into CachedWidget?

@alanpoon
Copy link
Contributor

alanpoon commented Oct 5, 2024

It seems like room_list is inside cached_widget.

rooms_list = <RoomsList> {}

It is not possible to differentiate room_type as property in the roomlist's draw_walk function as there can only be 1 instance: e.g

<CachedWidget>{ room_list = <RoomList>{ room_type: "people" } } <CachedWidget>{ room_list = <RoomList>{ room_type: "room" } } fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { self.room_type --> will always be "people". } @kevinaboos Is there a way to pass "room_type" property into CachedWidget?

I will try passing the property through context.

@aaravlu
Copy link
Contributor

aaravlu commented Oct 5, 2024

I will try passing the property through context.

How do you pass attributes through context? Could you please be more specific?
Thank you :)

@alanpoon
Copy link
Contributor

alanpoon commented Oct 5, 2024

<CachedWidget>{ field: "people", room_list = <RoomList>{ } } <CachedWidget>{ field: "room" room_list = <RoomList>{ } }
pub struct CachedWidget{ #[live] field: String }
cx.set_global(..)

pub struct CachedField(String); impl WidgetNode for CachedWidget { fn walk(&mut self, cx: &mut Cx) -> Walk { if let Some(widget) = &self.widget { cx.set_global(CachedField(self.field.clone())); widget.walk(cx) } else { self.walk } }

@alanpoon
Copy link
Contributor

alanpoon commented Oct 5, 2024

Screenshot 2024-10-05 221010
It seems like I can only render 2 similar cached widgets for text_input widget butnot the other widgets. For the other widgets, the second render is always missing.

Here is how I do cached widget for text_input widget.
in the live_design:

<CachedWidget> {
                        height: 30,
                        field: "people"
                        input1 = <TextInput> {
                            width: 100
                        }
                    }
                    button1 = <Button> {
                        text: "Hello world "
                        draw_text:{color:#f00}
                    }
                     <CachedWidget> {
                        height: 30,
                        field: "room"
                        input1 = <TextInput> {
                            width: 100
                        }
                    }
```
I added these lines before widget.walk in CachedWidget. 
```
impl WidgetNode for CachedWidget {
    fn walk(&mut self, cx: &mut Cx) -> Walk {
        if let Some(widget) = &self.widget {
            if cx.has_global::<CachedField>(){
                *cx.get_global::<CachedField>() = CachedField(self.field.clone());
            }else{
                cx.set_global(CachedField(self.field.clone()));
            }
            
            println!("set field {:?}",self.field.clone());
            widget.walk(cx)
```

```
impl Widget for TextInput
fn draw_walk(&mut self, cx: &mut Cx2d, _scope: &mut Scope, walk: Walk) -> DrawStep {
        let cf = cx.get_global::<CachedField>();
        let c= cf.0.clone();
        self.set_text(&c);

```
If the second rendering for cachedwidget works for other widgets, then we can use this method.

@aaravlu
Copy link
Contributor

aaravlu commented Oct 5, 2024

Could you leave your e-mail ? There's something personal I'd like to talk to you about.
: )

@alanpoon
Copy link
Contributor

alanpoon commented Oct 7, 2024

I created a draft PR #174. Currently, for some reasons, the program will hang when 2 portallists in RoomsList widget are drawn. So I thought I have to use cached_widget. It seems like cached_widget is being used after this issue has been created. So cached_widget might not solve the program's hang problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Looking for help from anyone!
Projects
Status: Ready
Development

No branches or pull requests

3 participants