Make invited room behavior prettier and sleeker #592#951
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the invited-room UX by updating the InviteScreen’s join-button visuals (enter-room icon → joining spinner → joined confirmation) and by automatically closing/hiding a room’s tab when it’s successfully left, preventing “dead” tabs from lingering.
Changes:
- Update InviteScreen join button iconography and joined-state text, and add a “Joining...” spinner view while join is in progress.
- Add a new
LeaveRoomResultActionhandler inAppto close any open tab for a room that was successfully left and hide it from the rooms list.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/home/invite_screen.rs |
Updates invite UI states: join icon, joining spinner view, and joined confirmation styling. |
src/app.rs |
Closes/hides room tabs on successful leave via LeaveRoomResultAction::Left. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| script_apply_eval!(cx, accept_button, { | ||
| draw_icon.svg: mod.widgets.ICON_CHECKMARK, | ||
| draw_icon.color: (COLOR_FG_ACCEPT_GREEN), | ||
| }); |
There was a problem hiding this comment.
yes, this is correct, you need to make this change.
kevinaboos
left a comment
kevinaboos
left a comment
There was a problem hiding this comment.
Hi @aneeshaa1, thank you for this contribution!
I left some comments about styling and ux behavior preferences.
First up, kindly test your code thoroughly before submitting it. I can tell that certain things won't work, e.g., the script_apply_eval invocation that copilot identified, and the general process of closing a room in mobile view mode (when robrix is shown in a narrow window). Please provide a screen recording as evidence of things working properly, in both desktop and mobile view mode.
Also, please disclose which AI agent or LLM workflow you used to generate this.
Thanks!
| // Shown in place of the `accept_button` while a join is in progress. | ||
| // A spinner inside a frame that mimics the disabled button's appearance. | ||
| joining_view := RoundedView { | ||
| visible: false | ||
| width: Fit, height: Fit | ||
| align: Align{x: 0.5, y: 0.5} | ||
| padding: 15, | ||
| spacing: 10, | ||
| flow: Right | ||
|
|
||
| show_bg: true | ||
| draw_bg +: { | ||
| color: (COLOR_BG_DISABLED) | ||
| border_radius: 4.0 | ||
| } | ||
|
|
||
| Label { | ||
| align: Align{x: 0.5, y: 0.5} | ||
| draw_text +: { | ||
| color: (COLOR_FG_DISABLED) | ||
| text_style: REGULAR_TEXT {font_size: 10} | ||
| } | ||
| text: "Joining..." | ||
| } | ||
|
|
||
| joining_spinner := BouncingDots { | ||
| margin: Inset{top: 1.0} | ||
| draw_bg.color: (COLOR_FG_DISABLED) | ||
| } | ||
| } |
There was a problem hiding this comment.
now that we have the LoadingSpinner widget, I vote that we should use that instead. You can follow the example of the 3 spinners used in account_settings.rs: showing a loading spinner next to the button while the join action is in progress.
Alternatively, you can keep this UI style of an imitation button, but just replace the BouncingDots with a LoadingSpinner.
| accept_button.set_enabled(cx, true); | ||
| accept_button.set_visible(cx, true); | ||
| joining_view.set_visible(cx, false); | ||
| joining_spinner.stop_animation(cx); |
There was a problem hiding this comment.
the benefit of using the LoadingSpinner is that you won't need to start/stop it.
| fn close_room(&mut self, cx: &mut Cx, room_id: &OwnedRoomId) { | ||
| let tab_id = LiveId::from_str(room_id.as_str()); | ||
| cx.widget_action( | ||
| self.ui.widget_uid(), | ||
| DockAction::TabCloseWasPressed(tab_id), | ||
| ); | ||
| enqueue_rooms_list_update(RoomsListUpdate::HideRoom { room_id: room_id.clone() }); | ||
| } |
There was a problem hiding this comment.
so, i can see that you duplicated this from the close_room_closure_opt below (in navigate_to_room()), which is fine, but you didn't dedup the code there to call this new function. Let's do that as well, please.
Also, did you test this? What happens in mobile view mode where there is no dock? Note that the other code that you copied this from (close_room_closure_opt) only works because a new room has already been selected, which is what drives the mobile UI behavior. AFAICT, this will do nothing in mobile view mode.
| script_apply_eval!(cx, accept_button, { | ||
| draw_icon.svg: mod.widgets.ICON_CHECKMARK, | ||
| draw_icon.color: (COLOR_FG_ACCEPT_GREEN), | ||
| }); |
There was a problem hiding this comment.
yes, this is correct, you need to make this change.
Closes #592
A few UX touch-ups to the invite screen: