-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
VIDCS-3040: There is a difference in publisher preview on waiting room and the meeting room #65
Conversation
…to cpettet/vidcs-2983-waiting-room-cut-off
…c.ts-snapshots/Waiting-page-UI-test-1-Electron-linux.png
…c.ts-snapshots/Waiting-page-UI-test-1-Google-Chrome-Fake-Devices-linux.png
…c.ts-snapshots/Waiting-page-UI-test-1-Microsoft-Edge-linux.png
…c.ts-snapshots/Waiting-page-UI-test-1-Opera-linux.png
…c.ts-snapshots/Waiting-page-UI-test-1-firefox-linux.png
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! great job
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but this needs testing on real devices to make sure it works as we expect 🙏
console.error('initPublisher error: ', err); | ||
publisherRef.current = initPublisher( | ||
undefined, | ||
{ insertDefaultUI: false, resolution: '1280x720' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we double check this is doing what we expect on real devices too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the test instructions to test on real devices. FWIW, it looked okay on my iOS device
looks good! |
What is this PR doing?
Description
For mobile devices, the preview publisher in the waiting room and the one displayed for subscribers in the meeting room don't match up. See screenshots below, notice the presence or absence of the shoe tower in the before shots.
Screenshots (after)
Screenshots (before)
How should this be manually tested?
Repro'ing the issue
develop
or go to the live linkVerifying the fix
Trying on real devices
What are the relevant tickets?
A maintainer will add this ticket number.
Resolves VIDCS-3040
Checklist
[✅] Branch is based on
develop
(notmain
).[✅] 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?