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

Accept Multiple video Resolutions and Canvas Recording #427

Closed
wants to merge 3 commits into from

Conversation

Forchapeatl
Copy link
Collaborator

Accept Multiple video Resolutions and Canvas Recording

3d0cdb6d-e9ca-4485-8e99-28eb011fae82.mp4

Hello @jywarren @TildaDares @cesswairimu and @stephaniequintana
I have

Questions

  1. The flow to update web resolution is
    CASE A
    a. Turn on Camera.
    b. Select resolution.
    or
    CASE B
    a. Select resolution,.
    b. Turn on Camera.
    The problem is on case B our presets modal will show after every resolution is selected. I propose keep the multiple resolution button disabled until our camera is turned on.

Optimization

The frames move much slower at higher resolutions.The resolution limit is subject to to the RAM of the computer. I will have to set a maximum resolution limit for each device. reference

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

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
  • code is in uniquely-named feature

branch and has no merge conflicts 📁

  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

- Included logic to accept multiple video resolutions.
- Included logic to record and download  canvas.
@gitpod-io
Copy link

gitpod-io bot commented Jun 24, 2022

Copy link

@cesswairimu cesswairimu left a comment

Choose a reason for hiding this comment

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

Awesome 🎉 🎉 this is great looking @Forchapeatl
On styling;- I think the buttons are too compact , what do you think? - this could be something you could improve on a follow up PR ; - maybe reducing the width of the buttons and adding some space between could help. Would like to hear your thoughts, Great job 🚀

@cesswairimu
Copy link

On resolution flow: - case A as you proposed seems smooth and I am pro that. Would like to hear thoughts from others. thanks

@Forchapeatl
Copy link
Collaborator Author

@cesswairimu thank you for the kind remarks. I will discuss with @stephaniequintana on the styling

@stephaniequintana
Copy link
Collaborator

Great job, @Forchapeatl!! You're really truckin' along, I love it!! Congrats on this:exclamation: :star2: :boom: :dizzy:

@jywarren
Copy link
Member

@Forchapeatl this is really cool! I agree with Cess that path A sounds good - i think it's OK to force people into that flow and it doesn't seem like it will prevent people from doing anything. Better to keep it simple by design!

I am wondering how we should best organize the UI here. I have a few questions - first, can we visually isolate/contain the new controls from other controls AND from each other, to simplify the view for the user, and second, in the code itself so that when the time comes, @stephaniequintana knows exactly what new code needs to be copied into the new UI?

@stephaniequintana do you have ideas on how to do this? I was thinking maybe we could... make them appear only when you click the video button... and make the record button appear as a button in the same row (and same style) as the camera button? Could we make the webcam/video/upload buttons disappear and replace them with the new controls like resolution and record? Could the record button be red? These are just a few ideas.

Hm, are we missing a commit here, though? I don't see the extra new HTML for the buttons the way I see in https://forchapeatl.github.io/infragram/index2.html - just wondering if we missed something.

I just want to say again this is really exciting, and great work!

@stephaniequintana
Copy link
Collaborator

I also agree with case A.

@stephaniequintana do you have ideas on how to do this? I was thinking maybe we could... make them appear only when you click the video button... and make the record button appear as a button in the same row (and same style) as the camera button? Could we make the webcam/video/upload buttons disappear and replace them with the new controls like resolution and record? Could the record button be red?

Currently, the new UI has the webcam/video/upload button as a dropdown that lists the resolutions. A few thoughts:

  • We can definitely disappear the camera and webcam buttons and use that space for the resolution/record buttons. My only concern is if the button is pressed by mistake or out of curiosity then the page may need to be reloaded in order to switch back to camera/image-upload (?) - I'll play around with both, disappearing the camera/webcam buttons and making room on the same row to the right of the video icon.
  • Does the resolution selection and record buttons need to be separate buttons? - Just curious. We might say "Begin recording in:" resButton1, resButton2... and have them turn red once hovered over? Just a thought...

so that when the time comes, @stephaniequintana knows exactly what new code needs to be copied into the new UI?

I'm wondering about this myself. Not necessarily in what will need to be copied, but the best approach. My unexpected travel last week put me further behind that I'm comfortable being. On that note, I am home now and well rested and will work diligently this week to catch up. I'm working on finishing connecting the functions and am hoping @Forchapeatl and I can move forward with working on the same html file.

I've connected my index2.html to all of the existing js files. I know last week we discussed adding a separate infragram2.js, but I think we should use and add to what currently works. On the approach of incorporating @Forchapeatl's additions, I will go ahead and incorporate her html/UI additions into mine and once her js/functions are merged all should work 🙏
Please let me know if you have any ideas on a better approach.

@Forchapeatl
Copy link
Collaborator Author

Forchapeatl commented Jun 28, 2022

Hello @stephaniequintana sorry for the delay. I have updated the commits with both the dist/infragram.js and index.ntml files.
I will incorporate the newUI at my end too (under this PR). I think the resolution and record buttons should be different. Let's show the record button when Webrtc returns true. I will attach my functionality to the newUI. so that we can be in sync. Thank you.

@jywarren
Copy link
Member

jywarren commented Jul 2, 2022

if the button is pressed by mistake or out of curiosity then the page may need to be reloaded in order to switch back to camera/image-upload (?) - I'll play around with both, disappearing the camera/webcam buttons and making room on the same row to the right of the video icon.

Good question. I think needing to reload the site isn't the worst, especially in the v1 interface. On the other hand, in v2, the process might work differently to avoid this, or we could imagine having an x button to close this new little pane. I trust you two to think through the flow and figure something out. You might also ask other Outreachy/GSoC fellows to try it and see what seems natural or confusing to them!

Does the resolution selection and record buttons need to be separate buttons? - Just curious. We might say "Begin recording in:" resButton1, resButton2... and have them turn red once hovered over? Just a thought...

I think this sounds promising, but might also do well with testing -- especially in mobile where there is no hover effect (touchscreen). Haha someday we'll be able to detect a hover-finger-above-screen... but not yet.

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.

GSoC Infragram.org full-screen UI and video upload Discussion and Planning
4 participants