WIP the desktop ui for room details pane (room info, room members) #119 #213
Conversation
tyreseluo
changed the title
tyreseluo
changed the title
b1oxKOA0pD.mp4 |
There was a problem hiding this comment.
Thanks, great start! I've left a review at your request even though the PR is marked as a draft.
From a UI design perspective, I feel like this is a bit clunky because it uses a lot of vertical space, which is particularly egregious on small screens. However, we should reach out to an actual UI designer for better ideas & informed opinions, like Sebastian from Makepad.
However, it's ok for now since it enables us to test the actual UX functionality & behavior.
There was a problem hiding this comment.
let's choose a different icon. this does not look like a "leave room" icon to me, it barely even looks like an open door... it's too abstract.
There was a problem hiding this comment.
please place this and all icon files in the proper subdirectory within resources/.
| // Handle the room info tab being clicked. | ||
| if self.button(id!(room_info)).clicked(&actions) { | ||
| self.show_room_info_pane( | ||
| cx, | ||
| &room_details_pane, | ||
| ); | ||
| } | ||
|
|
||
| // Handle the room members tab being clicked. | ||
| if self.button(id!(room_members)).clicked(&actions) { | ||
| self.show_room_members_pane( | ||
| cx, | ||
| &room_details_pane, | ||
| ); | ||
| } |
There was a problem hiding this comment.
for these, don't you actually want to toggle the room info/members pane, instead of unconditionally showing it?
That is, if the room_info tab button is clicked when the room info tab is already open, the user would likely expect that pane to be closed.
| if pane.is_currently_shown(cx) { | ||
| pane.handle_event(cx, event, scope); | ||
| } | ||
|
|
||
| if room_details_pane.is_currently_shown(cx) { | ||
| room_details_pane.handle_event(cx, event, scope); | ||
| } | ||
| } | ||
|
|
||
| self.view.handle_event(cx, event, scope); | ||
|
|
There was a problem hiding this comment.
this isn't quite correct. We only want to have the underlying view handle the event if it doesn't require visibility AND none of the panes are currently shown. You've changed it to handle the event unconditionally.
| @@ -12,6 +12,8 @@ pub mod login; | |||
| pub mod home; | |||
| /// User profile info and a user profile sliding pane. | |||
| mod profile; | |||
| /// Room details info and room members sliding pane. | |||
| mod room; | |||
There was a problem hiding this comment.
please use a more descriptive name. room is widely used in the Matrix SDK itself and isn't a good name for this.
room_info_sliding_pane would be my recommendation.
| fn handle_actions(&mut self, _cx: &mut Cx, actions:&Actions, _scope: &mut Scope) { | ||
|
|
||
| if self.button(id!(copy_link_to_room_button)).clicked(actions) { | ||
| log!("Copy link to room button clicked"); | ||
| } | ||
|
|
||
| if self.button(id!(invite_button)).clicked(actions) { | ||
| log!("Invite button clicked"); | ||
| } | ||
|
|
||
| if self.button(id!(leave_room_button)).clicked(actions) { | ||
| log!("Leave room button clicked"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl RoomInfoPane { | ||
| pub fn set_room_info(&mut self, room_info: RoomInfo) { | ||
| log!("Setting room info: {:?}", room_info); | ||
| } |
There was a problem hiding this comment.
i assume this is still a WIP?
kevinaboos
added
the
waiting-on-author
Fixes: #119