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

Disables Take a Photo Button and File Chooser While Camera is Active #1357

Merged
merged 2 commits into from Dec 16, 2019
Merged

Disables Take a Photo Button and File Chooser While Camera is Active #1357

merged 2 commits into from Dec 16, 2019

Conversation

rcya1
Copy link

@rcya1 rcya1 commented Dec 14, 2019

Fixes #1321

This disables the other two buttons after the "Take a Photo" button is pressed and reenables them once the camera is closed.

Before "Take a Photo" is pressed and after camera is closed:
Screen Shot 2019-12-13 at 9 04 17 PM

After "Take a Photo" is pressed (I covered the webcam with my finger which is why the camera stream appears all black):
Screen Shot 2019-12-13 at 9 04 28 PM

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/is-reviewers for help, in a comment below
  • Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!

@rcya1
Copy link
Author

rcya1 commented Dec 14, 2019

@publiclab/is-reviewers Please review this, thank you!

@codecov
Copy link

codecov bot commented Dec 14, 2019

Codecov Report

Merging #1357 into main will decrease coverage by 0.15%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1357      +/-   ##
==========================================
- Coverage   66.57%   66.41%   -0.16%     
==========================================
  Files         125      125              
  Lines        2546     2552       +6     
  Branches      397      397              
==========================================
  Hits         1695     1695              
- Misses        851      857       +6
Impacted Files Coverage Δ
src/ui/SetInputStep.js 12.9% <0%> (-1.39%) ⬇️

@keshav234156
Copy link
Member

@Ryan10145 Can you please upload a gif of the changes made. After the change "click picture" and "close" buttons were not working for me and also the camera was not able not.

@rcya1
Copy link
Author

rcya1 commented Dec 14, 2019

@keshav234156 I'm not sure why that's happening, as it works fine on my machine, as shown in the GIF below. Can you check to make sure that the webcam works properly before the changes? Even before my changes, I had an issue before where on my computer without a webcam, the webcam stream wouldn't show (because it didn't exist) and also the buttons wouldn't work, which sounds similar to your situation. To test and incorporate these changes, I switched to a laptop which did have a webcam. I was going to further investigate and if necessary, create an issue/pull request to fix this problem.

Dec-14-2019 10-49-48
I blocked the webcam again with my thumb, although I moved it back and forth to show that it is a live video.

@@ -9,7 +9,7 @@ module.exports = function ImportImageModuleUi(step, ui) {
// add a file input listener
var dropZone = '\
<div class="dropzone import-image-zone" id="' + dropzoneId + '">\
<p>\
<p id="dropzone-text">\
Copy link
Member

Choose a reason for hiding this comment

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

I think this change was not needed.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry about that, that was one of my testing changes that I forgot to remove before committing. You're correct, that change was not needed. Thanks for pointing that out!

Copy link
Member

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

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

LGTM!

@jywarren jywarren merged commit 3d4025f into publiclab:main Dec 16, 2019
@jywarren
Copy link
Member

Thanks, all!!! 🎉

jywarren pushed a commit that referenced this pull request Dec 16, 2019
…1357)

* Disables Other Buttons While Camera is Active

* Remove dropzone-text id from ImportImage UI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable take a photo and choose a file button while camera is open
4 participants