publiclab / infragram

A minimal core of the Infragram.org project in JavaScript
https://infragram.org/sandbox/
GNU General Public License v2.0
44 stars 166 forks source link

Added Streaming functionality for iOS Browsers #442

Closed Forchapeatl closed 2 years ago

Forchapeatl commented 2 years ago

Hosted on https://forchapeatl.github.io/infragram/indexs.html

gitpod-io[bot] commented 2 years ago

jywarren commented 2 years ago

Can you check if any of the changes to the src files adversely affected v1 functionality? Thank you!

On Fri, Aug 5, 2022 at 7:21 PM FORCHA @.***> wrote:

@.**** commented on this pull request.

In package.json https://github.com/publiclab/infragram/pull/442#discussion_r939442510:

@@ -25,11 +25,9 @@ "bootstrap": "~3.4.0", "bootstrap5": "npm:bootstrap@^5.1.3", "font-awesome": "~4.7.0",

  • "getusermedia-js": "~1.0.0",

@jywarren https://github.com/jywarren , package.json has been fixed. Please may I merge?

— Reply to this email directly, view it on GitHub https://github.com/publiclab/infragram/pull/442#discussion_r939442510, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6JYOY2CBLIH6REQPM5LVXWOWVANCNFSM55XEYOFA . You are receiving this because you were mentioned.Message ID: @.***>

stephaniequintana commented 2 years ago

FANTASTIC work, @Forchapeatl! Kudos!!! 🚀 🎆

Forchapeatl commented 2 years ago

sorry about that @jywarren it affects the functionality of camera.js a whole lot. As the bootstrap modal is replaced by offcanvas and WebcamVideo element is not created by WebRTC. Should I create a seperate cameraV2.js file ?

jywarren commented 2 years ago

sorry about that @jywarren it affects the functionality of camera.js a whole lot. As the bootstrap modal is replaced by offcanvas and WebcamVideo element is not created by WebRTC. Should I create a seperate cameraV2.js file ?

It's ok! @stephaniequintana would you like to explain how we could use the options flag to switch behaviors between the two versions?

Forchapeatl commented 2 years ago

Hello @jywarren the options.version condition has been included to the camera.js and dist/infragram.js files. Please have a look at the video below . hosted on (version 2 )https://forchapeatl.github.io/infragram/indexs.html and (version 1) https://forchapeatl.github.io/infragram/index.html . May I merge this PR ?

https://user-images.githubusercontent.com/24577149/183268935-86ef963f-2f03-47b5-9887-bc6a909d0bc9.mp4

welcome[bot] commented 2 years ago

Congrats on merging your first pull request! 🙌🎉⚡️ Your code will likely be published to http://infragram.org in the next few days. Now that you've completed this, you can help someone else take their first step! See: Public Lab's coding community!

jywarren commented 2 years ago

So, one more thing @Forchapeatl -- here, are you generating the dist/infragram.js and then renaming it dist/infragram2.js? If all the version differences are managed by the options.version conditionals now, can't we begin leaving it as dist/infragram.js instead of infragram2.js, since both versions will be able to run off the same file?

And thank you for merging this and moving it along -- i think sorry it might have needed just one more review before doing so but it's not a big deal since you did confirm that the previous version was working fine!

However if you open a new PR where we just use dist/infragram.js we'll have to be extra sure all features work in both versions since they'll all be running off the same file from there onward. Make sense?

Thanks!!!

jywarren commented 2 years ago

Also just cc'ing @stephaniequintana can you and @Forchapeatl just put a 👍 on the above so I know we're all coordinated? Thank you! Exciting!!!!

jywarren commented 2 years ago

And thanks for your patience this week as I'm teaching long hours so less able to reply quickly! Definitely continue relying on reviews from each other and other mentors as well!