Skip to content

Conversation

@OscarFava
Copy link
Contributor

@OscarFava OscarFava commented Aug 6, 2025

What is this PR doing?

This PR is adding the capability to add custom images that will be used to replace the background of the publisher.

How should this be manually tested?

1- Enter to waiting room, click "Background Effects" button on the bottom corner.
1.1 - Test tab "backgrounds". Select different backgrounds that will be applied on the publisher. If you "cancel", that background will not be applied to the main publisher, if you click "apply", it will be applied.
1.2 - Click tab "Add Background" and add a custom image. Then, do same as step 1.1 with that custom image.
2- Enter to meeting room and click on the button next to the camera (video devices) and then click "Background Effects".
2.2 - Do the same as 1.1 and 1.2

What are the relevant tickets?

A maintainer will add this ticket number.

Resolves VIDSOL-105

Checklist

[ ] Branch is based on develop (not main).
[ ] Resolves a Known Issue.
[ ] If yes, did you remove the item from the docs/KNOWN_ISSUES.md?
[ ] Resolves an item reported in Issues.
If yes, which issue? Issue Number?

@OscarFava OscarFava changed the title Tabs layout VIDSOL-105: Background Replacement Support for Web VERA - Custom Images Aug 6, 2025
Copy link
Contributor

@cpettet cpettet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far! Just have a few comments/questions. Let me know what you think!

mode: 'meeting' | 'waiting';
};

const BackgroundEffectsLayout = ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add jsdoc for this component?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we're still missing the jsdoc here but the comment was shown as resolved 🙏

@OscarFava OscarFava requested a review from cpettet September 23, 2025 08:16
cpettet
cpettet previously approved these changes Sep 23, 2025
Copy link
Contributor

@cpettet cpettet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Great job! :shipit:

behei-vonage
behei-vonage previously approved these changes Sep 23, 2025
Copy link
Contributor

@behei-vonage behei-vonage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! great job on this one!

@dwivedisachin
Copy link
Contributor

dwivedisachin commented Sep 25, 2025

Hey @OscarFava, It looks 🤩
I found a few issues/nits.
We can fix these in a followup PR or in the same one. Your choice. 🙂

BG replacement desktop:

  • No tool tip on Delete BG icon
  • After a few Backgrounds the backgrounds are added but not applied (may be after hitting the max)
  • On the waiting room, BG replacement on changing the Backgrounds to add Backgrounds the popup size changes which looks weird, may be it should be fixed and a scroll should appear when too many BGs

Mobile view:

  • On super small screen like iphone SE the cancel and apply buttons are not fully visible

@OscarFava OscarFava dismissed stale reviews from behei-vonage and cpettet via 81fa0d3 September 29, 2025 10:22
Copy link
Contributor

@behei-vonage behei-vonage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job on this one! left a few more comments. thanks 🙏

mode: 'meeting' | 'waiting';
};

const BackgroundEffectsLayout = ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we're still missing the jsdoc here but the comment was shown as resolved 🙏

@dwivedisachin
Copy link
Contributor

Tested LGTM!! 🚀
Publisher takes a bit longer to show up when BG replacement is on, but let's see how it looks when we dogfood it.
🚀

Copy link
Contributor

@cpettet cpettet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far! Just have a few comments/questions. Let me know what you think!

@OscarFava OscarFava requested a review from cpettet October 1, 2025 09:56
VZaphod
VZaphod previously approved these changes Oct 1, 2025
Copy link

@VZaphod VZaphod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@VZaphod VZaphod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 1, 2025

Copy link

@marinaserranomontes marinaserranomontes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@OscarFava OscarFava merged commit 059a159 into develop Oct 1, 2025
10 of 13 checks passed
@OscarFava OscarFava deleted the vidsol-105/background-replacement-custom-images branch October 1, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants