Feature/Host Video in canvas

[testradav]

issue #460

[darioalessandro]

@testradav nice dude!! this is awesome

[darioalessandro]

@testradav amazing PR mate!! thank you for your contribution.

I found a couple of style changes related to the forked codebase that I do not think should be added here.

After that we can go ahead and ship this

[testradav]

@testradav amazing PR mate!! thank you for your contribution.

I found a couple of style changes related to the forked codebase that I do not think should be added here.

After that we can go ahead and ship this

yes, oops, I forgot I had cleaned up the landing page :)

[testradav]

I am figuring out a bug where the host video stops showing (even though the camera is still on) whenever the canvas refreshes (eg. when someone else joins the meeting)

[darioalessandro]

I am figuring out a bug where the host video stops showing (even though the camera is still on) whenever the canvas refreshes (eg. when someone else joins the meeting)

The bug might be that the canvas changes names? videocall-client right now uses the unique canvas id to know which canvas to render to.

[darioalessandro]

I did another pass and I think I found what is causing render issues:

Summary

Great work implementing the host video in the canvas grid! The feature works well and improves the UX. However, there are some architectural and code quality issues that need addressing.

What Works Well

• Successfully displays host video alongside peer tiles
• Proper integration with existing grid layout
• Change name functionality is working
• CSS updates for the floating header look good

Issues to Address

1. Overly Complex Callback Pattern

Problem: The RegisterHostToggleName message and callback registration is unnecessarily complex.

// Current approach in attendants.rs (lines 84, 584-587)
Msg::RegisterHostToggleName(Callback<MouseEvent>)
// Then stored in state and passed down

Solution: Pass the callback directly through props instead of registering it via messages. Simpler data flow.

2. String Manipulation Hack

Problem: Manually appending " (You)" to identify the host (line 607):

display_peers_vec.insert(0, my_email.clone() + " (You)");

Solution: Create a proper struct like PeerInfo { id: String, display_name: String, is_self: bool } instead of string manipulation.

3. Video Element ID Inconsistency

The PR changes the host video to use id="webcam" but this ID is hardcoded in multiple places. Should be:

• Configurable via props/constants
• Or use a consistent naming scheme like other peers

4. Duplicate Conditional Logic

In canvas_generator.rs, the host vs peer video rendering logic is duplicated in two places (full-bleed and grid modes). Extract to a helper function:

fn render_video_element(is_host: bool, key: &str) -> Html {
    if is_host {
        html!{ <video class="self-camera" autoplay=true id={"webcam"} ...> }
    } else {
        html!{ <UserVideo id={key.clone()} hidden={false}/> }
    }
}

5. Code Quality Issues

• Commented CSS: Lines in global.css and style.css should be removed if not needed
• Hackish check: Line 42 in canvas_generator.rs: is_video_enabled_for_peer || is_host - should be explicit
• Peer list integration: The self-entry was removed from peer_list.rs but should show there too for consistency

6. Log Statement

Line 144 in peer_tile.rs - remove or change to debug level before merging:

log::info!("YEW-UI: Rendering Peer Tile: {} {}", ...)

Recommendation

Please refactor to address items 1-5 before merging. The feature works, but the architecture can be much cleaner. Happy to review again once updated!

[testradav]

Thanks for the detailed and thorough review. I blame coding RUST with AI :)
I will make the changes suggested

[darioalessandro]

Hey @testradav I use AI too! Yes it does produce bad code sometimes, we should share rules to make our dev process better, so you use cursor or clause code?

[darioalessandro]

Hey @testradav can you put this behind a feature flag? some people like the facetime-whatsapp behavior. Making it a FF will make it optional (I do like this btw)