Enhance popup notifications with colors, severity icons, and a progress bar

[alanpoon]

Fixes #517

Screen.Recording.2025-06-23.at.1.58.54.PM.mov

Screen.Recording.2025-06-23.at.1.59.45.PM.mov

[ZhangHanDong]

1. There is a bug:

The logic of using removed_indices.iter() to batch delete popup items has a problem:

Code flow

 1. Collection phase:let mut removed_indices = Vec::new();
for (index, (view, close_popup_timer)) in self.popups.iter_mut().enumerate() {
    if close_popup_timer.is_event(event).is_some() {  // Check if the timer has expired
        removed_indices.push(index);  // Collect indices to be removed
    }
}

1. Deletion phase:

for &i in removed_indices.iter() {
   self.popups.remove(i);  // Remove one by one
}

Key issue: Index invalidation

The problem is:
• ‎⁠Vec::remove(i)⁠ deletes the i-th element and shifts all subsequent elements forward
• This causes all subsequent indices to become invalid

Example to illustrate the problem

Suppose there are 5 popups [A, B, C, D, E], and you need to delete indices [1, 3] (i.e., B and D):// Initial state: [A, B, C, D, E] Indices: [0, 1, 2, 3, 4]

// removed_indices = [1, 3]

// First removal remove(1): Remove B
// Result: [A, C, D, E]  Indices: [0, 1, 2, 3]

// Second removal remove(3): Intended to remove D, but actually removed E!
// Result: [A, C, D]  ❌ Wrong! D was not deleted

2. Suggestion: Move join_leave_room_modal.rs to the src/room/ directory, since it specifically handles room-related join/leave operations.

[alanpoon]

1. There is a bug:

The logic of using removed_indices.iter() to batch delete popup items has a problem:

Code flow

 1. Collection phase:let mut removed_indices = Vec::new();
for (index, (view, close_popup_timer)) in self.popups.iter_mut().enumerate() {
    if close_popup_timer.is_event(event).is_some() {  // Check if the timer has expired
        removed_indices.push(index);  // Collect indices to be removed
    }
}

1. Deletion phase:

for &i in removed_indices.iter() {
   self.popups.remove(i);  // Remove one by one
}

Key issue: Index invalidation

The problem is: • ‎⁠Vec::remove(i)⁠ deletes the i-th element and shifts all subsequent elements forward • This causes all subsequent indices to become invalid

Example to illustrate the problem

Suppose there are 5 popups [A, B, C, D, E], and you need to delete indices [1, 3] (i.e., B and D):// Initial state: [A, B, C, D, E] Indices: [0, 1, 2, 3, 4]

// removed_indices = [1, 3]

// First removal remove(1): Remove B
// Result: [A, C, D, E]  Indices: [0, 1, 2, 3]

// Second removal remove(3): Intended to remove D, but actually removed E!
// Result: [A, C, D]  ❌ Wrong! D was not deleted

2. Suggestion: Move join_leave_room_modal.rs to the src/room/ directory, since it specifically handles room-related join/leave operations.

Fix with a break in loop as there cannot be a same event for two different timer.

[kevinaboos]

I spoke with Rik about which shader states should be uniforms vs. instances. In your existing code here, only display_progres_bar needs to be an instance -- everything else can be a uniform.

               progress_bar = <View> {
                    width: Fill,
                    height: Fill,
                    show_bg: true,
                    draw_bg: {
                        instance border_radius: 2.,
                        instance border_size: 1.0,
                        instance progress_bar_color: (COLOR_AVATAR_BG_IDLE),
                        instance progress_bar_background_color: (COLOR_DISABLE_GRAY),
                        instance display_progress_bar: 1.0
                        uniform anim_time: 0.0,
                        uniform anim_duration: 2.0,
...

Please apply that same logic to the rest of your shader code. For example, direction, border_radius, and probably others can be uniforms instead of instances. In general, instances are more expensive and should only be used for things that apply to many different items being concurrently drawn.

[kevinaboos]

@alanpoon In the future, we will also want to support a popup notification with arbitrary content. For example, I'd like to use it for in-app notifications of messages in other rooms, so we'll want to be able to display a Room Name string, the Room Avatar, and the HTML content of a message.

You can add that in now, or we can save that for later. But just keep that in mind while you're implementing the new design of this widget, such that we can customize it later.

[alanpoon]

right_to_btm_popuplist.mov

top_to_bottom.mov

• Added 5 enum for PopupKind.
• {} -> Right To Left Progress bar
• {} -> Top to bottom Progress bar.

I don't think I can do arbitrary content for Popup list without using Closure.

[kevinaboos]

• Added 5 enum for PopupKind.
• {} -> Right To Left Progress bar
• {} -> Top to bottom Progress bar.

Where did that requirement come from? I don't recall anyone needing multiple different kinds of progress timer bars — just the one from right-to-left on the bottom edge of the view would be sufficient — but I suppose it's fine to have different options.

I don't think I can do arbitrary content for Popup list without using Closure.

I think you're overthinking it from a Rust language perspective. This can be implemented just lIke a Modal, in which the inner "content" view can be set by the user. Note that you don't have to do that in this PR, we can keep this one simple.

[alanpoon]

• Added 5 enum for PopupKind.
• {} -> Right To Left Progress bar
• {} -> Top to bottom Progress bar.

Where did that requirement come from? I don't recall anyone needing multiple different kinds of progress timer bars — just the one from right-to-left on the bottom edge of the view would be sufficient — but I suppose it's fine to have different options.

I don't think I can do arbitrary content for Popup list without using Closure.

I think you're overthinking it from a Rust language perspective. This can be implemented just lIke a Modal, in which the inner "content" view can be set by the user. Note that you don't have to do that in this PR, we can keep this one simple.

I've extrapolated your suggestion of making the progress bar a separate view rather than embedded into shader logic. Since it is a separate view, there should be some examples showing how to customize or reposition it. If we were to implement the arbitrary content in the Popup list like the Modal widget, then I suppose we have to remove the SegQueue and store the PopupNotification Reference into Global Ref.

[kevinaboos]

Thanks, things are looking pretty good. I left a few minor comments.

I'm not sure about the implementation of custom views within a popup, but that's fine for now since nobody is using it. Once we start using that functionality, then we can adjust the API to suit our needs.

[kevinaboos]

Left a few comments.

The "info" icon you've picked is a lot different than all the other icons used in Robrix. We're using a style that is more like this one: https://www.svgrepo.com/svg/522904/info-circle. Please replace it with something more minimalist like that.

[kevinaboos]

Did you perhaps forget to push/commit something? Seems like you didn't change the info icon to what I previously suggested, nor did you address all the ALL_CAPS formatting (i still see: CROSS_ICON, INFO_ICON, WARNING_ICON, PROGRESS_BAR, MAIN_CONTENT, LEF_SIDE_VIEW, CLOSE_BUTTON_VIEW).

Generally, (for future notice), we want something like this for the severity of error levels:

• Success: green, checkmark
• Error: red, with a stop sign (or a forbidden icon to indicate cancelation). An "X" should be used to indicate closing something, not to show an error.
• Warning: yellow, with yield sign (triangle) and/or exclamation point
• Info: blue, i within a circle

[alanpoon]

Did you perhaps forget to push/commit something? Seems like you didn't change the info icon to what I previously suggested, nor did you address all the ALL_CAPS formatting (i still see: CROSS_ICON, INFO_ICON, WARNING_ICON, PROGRESS_BAR, MAIN_CONTENT, LEF_SIDE_VIEW, CLOSE_BUTTON_VIEW).

Generally, (for future notice), we want something like this for the severity of error levels:

• Success: green, checkmark
• Error: red, with a stop sign (or a forbidden icon to indicate cancelation). An "X" should be used to indicate closing something, not to show an error.
• Warning: yellow, with yield sign (triangle) and/or exclamation point
• Info: blue, i within a circle

Fixed.

[kevinaboos]

Thanks, LGTM!