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

Fix switch camera #6613

Merged
merged 4 commits into from
Oct 19, 2023
Merged

Fix switch camera #6613

merged 4 commits into from
Oct 19, 2023

Conversation

ryaplots
Copy link
Contributor

@ryaplots ryaplots commented Oct 3, 2023

Summary

References #6529

Changes

  • Use a state for video mode
test.mp4

Testing

  1. Log in the console on a mobile device
  2. Go to register end device
  3. Click on scan end device QR code
  4. See the switch button at the bottom of the video
  5. Click and see the camera changing

Notes for Reviewers

Not using stream for conditions because at some point in time it is an empty object, which prevents comparing it's contents and makes it unreliable for the switch.

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • The steps/process to test this feature are clearly explained including testing for regressions.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@ryaplots ryaplots added the c/console This is related to the Console label Oct 3, 2023
@ryaplots ryaplots added this to the v3.28.0 milestone Oct 3, 2023
@ryaplots ryaplots self-assigned this Oct 3, 2023
@github-actions github-actions bot added ui/web This is related to a web interface and removed c/console This is related to the Console labels Oct 3, 2023
@mjamescompton
Copy link
Contributor

Is the clicking allow each time because of your set up or do you have to do that per camera?

@ryaplots
Copy link
Contributor Author

ryaplots commented Oct 4, 2023

Is the clicking allow each time because of your set up or do you have to do that per camera?

I think it's my setup

@mjamescompton
Copy link
Contributor

I have a feeling its every time you run await navigator.mediaDevices.getUserMedia({ it resets the the user permissions.

@ryaplots
Copy link
Contributor Author

ryaplots commented Oct 4, 2023

I have a feeling its every time you run await navigator.mediaDevices.getUserMedia({ it resets the the user permissions.

Might be. Let me test.

@kschiffer
Copy link
Contributor

Cool! Can we put the button on top of the camera stream instead? Right now it's causing this black bar at the bottom.
Also, can we fix the missing margin between the two "Cancel" and "Apply" button?

Copy link
Contributor

@mjamescompton mjamescompton left a comment

Choose a reason for hiding this comment

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

Could not get the switch to work on either iPhone 14 or Samsung Galaxy A10

Screenshot 2023-10-13 at 16 09 12

Nothing happens on the iPhone and above happens on the samsumg on chrome when I click the button

Copy link
Contributor

@kschiffer kschiffer left a comment

Choose a reason for hiding this comment

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

LGTM, though haven't tested.

Copy link
Contributor

@mjamescompton mjamescompton left a comment

Choose a reason for hiding this comment

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

Ok,

Iphone 14 pro - chrome and safari - testing

  • switch camera button still does not show on first render of the modal
  • switching the camera works once to go from back camera to front
  • but clicking the button again does nothing

I'm thinking we should put a component in between two we currently have. This one would specifically handle requesting the stream etc and includes the switching button, then we re-render the a component

@ryaplots ryaplots modified the milestones: v3.28.0, v3.28.1 Oct 19, 2023
@mjamescompton mjamescompton self-requested a review October 19, 2023 11:16
Copy link
Contributor

@mjamescompton mjamescompton left a comment

Choose a reason for hiding this comment

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

Still broken but going ahead anyway.

I don't have an android phone to test, but I believe the switch button not rendering will be there as well, so I don't think anyone will see it anyway

@ryaplots ryaplots merged commit c69cb8c into v3.28 Oct 19, 2023
@ryaplots ryaplots deleted the fix/camera-switch-mobile branch October 19, 2023 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui/web This is related to a web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants