Skip to content
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

Ahoyapps 495 switch camera #152

Merged
merged 11 commits into from
Apr 15, 2020
Merged

Ahoyapps 495 switch camera #152

merged 11 commits into from
Apr 15, 2020

Conversation

timmydoza
Copy link
Contributor

@timmydoza timmydoza commented Apr 14, 2020

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

JIRA link(s):

Description

This PR adds an icon that allows mobile users to toggle between their front and rear facing cameras. Users can do this before or after they join a room.

IMG_0003

Burndown

Before review

  • Updated CHANGELOG.md if necessary
  • Added unit tests if necessary
  • Updated affected documentation
  • Verified locally with npm test
  • Manually sanity tested running locally
  • Included screenshot as PR comment (if needed)
  • Ready for review

Before merge

  • Got one or more +1s
  • Re-tested if necessary

@@ -35,7 +35,8 @@ export default function VideoTrack({ track, isLocal, priority }: VideoTrackProps
}, [track, priority]);

// The local video track is mirrored.
const style = isLocal ? { transform: 'rotateY(180deg)' } : {};
const isFrontFacing = track.mediaStreamTrack.getSettings().facingMode !== 'environment';
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about track.mediaStreamTrack.getSettings().facingMode === 'user';? Because if facingMode is left or right, this evaluates to true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

facingMode is a property that seems to exist only on mobile devices. So isFrontFacing would be false when using the app on my laptop, and the video would not be mirrored when it should be.

Also, according to MDN, facingMode of left and right represents cameras that are front facing, just pointed over the user's left or right shoulder. I can't think of any devices with these cameras, but we would want isFrontFacing to be true in those cases.

Tim Mendoza added 2 commits April 15, 2020 15:14
@timmydoza timmydoza merged commit 718afe8 into master Apr 15, 2020
@timmydoza timmydoza deleted the AHOYAPPS-495-switch-camera branch April 15, 2020 23:34
jasonpennington5308 added a commit to jasonpennington5308/twilio-video-app-react that referenced this pull request Apr 16, 2020
Ahoyapps 495 switch camera (twilio#152)
uriasn pushed a commit to Mastermind-com/twilio-video-app-react that referenced this pull request Oct 6, 2020
* create FlipCameraButton component and add to menubar

* Pass facingMode to GetLocalVideoTracks

* Only mirror video when front-facing

* Add low priority to video publish

* Add comment

* Fix tests

* Add tests for FlipCameraButton

* Rename variable

* Update comment
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.

None yet

2 participants