-
Notifications
You must be signed in to change notification settings - Fork 733
Video 6578 display all camera tracks #585
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
Conversation
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.
This looks good! 🎉 Just need to add one more console.error()
in src/components/ChatProvider/index.tsx
@@ -32,7 +32,8 @@ export const ChatProvider: React.FC = ({ children }) => { | |||
window.chatClient = client; | |||
setChatClient(client); | |||
}) | |||
.catch(() => { | |||
.catch(e => { | |||
console.error(e); |
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 think we will need to add another console.error(e)
to line 76
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.
looks good!
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.
Looks good. Confirmed that there aren't any more instances of track.name.includes('camera')
👍
Contributing to Twilio
Pull Request Details
JIRA link(s):
Description
This PR addresses an interoperability issue between this app and Twilio's iOS and Android quickstart apps. The issue was that this app was not displaying video tracks because the tracks were not correctly labeled. If the track name did not include 'camera' (all lowercase), then the track would not be displayed. This means that unlabeled tracks, or tracks labeled as 'Camera' (uppercase 'C') would not be displayed.
This PR removes the requirement that video tracks have a name in order to be displayed. Screen share tracks must still contain the word 'screen' in order to be treated as screen share tracks.
Additionally, this PR makes the following changes:
Burndown
Before review
npm test
Before merge