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

ScreenCapture plugin for screenshots / screen recordings #2132

Merged
merged 13 commits into from
Apr 21, 2020

Conversation

arturi
Copy link
Contributor

@arturi arturi commented Mar 15, 2020

New @uppy/screen-capture plugin that adds screenshots / screen recordings functionality to Uppy, thanks to @jukakoski.

},
"peerDependencies": {
"@uppy/core": "^1.0.0"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add

"publishConfig": {
  "access": "public"
}

so npm doesn't try to create a private package once this is published for the first time (which would fail the lerna publish midway)

@arturi
Copy link
Contributor Author

arturi commented Apr 13, 2020

I think there should be a prop (in plugin) to tell if user agent is mobile or not as getDisplayMedia doesn't work in mobile. In Samsung S8 getDisplayMedia was true even thought it doesn't support it #1479 (comment)

I think we should rely on getDisplayMedia, because it’s Samsung S8’s fault that they claim to support the feature, but don’t. We could specifically blacklist devices that don’t support screen capture, but if we block all mobile devices, we’ll block iOS too, for instance, which does support it? And then more will follow as support improves?

@goto-bus-stop
Copy link
Contributor

goto-bus-stop commented Apr 20, 2020

hmm does anyone of us have a samsung S8 device to try what happens if you do try to record there? 🤔

@jukakoski
Copy link
Contributor

I have ;)
At the time I tested it the issue was that in UI the button to capture screen was visible although it's not supported. I guess this API won't be coming for mobile devices as using it could be quite difficult for users. In supported there is support only for ios13. Not sure which medias are supported though.
https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getDisplayMedia

@arturi
Copy link
Contributor Author

arturi commented Apr 21, 2020

Added a check for MediaRecorder, and it seems to have solved the issue with Safari on MacOS, so maybe it also helps with Galaxy. Gonna proceed with merging this, we’ll release as beta and fix bugs as we go on. Thanks a ton @jukakoski for working on this!

@arturi arturi merged commit a6c8ef1 into transloadit:master Apr 21, 2020
@@ -141,7 +141,6 @@ fi_FI.strings = {
streamActive: 'Jako aktiivinen',
streamPassive: 'Jako passiivinen',
micDisabled: 'Käyttäjä on estänyt mikrofonin',
micIsOn: 'Mikrofoni on päällä',
Copy link
Member

Choose a reason for hiding this comment

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

Intended @arturi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, half of the strings were actually unused in the UI. Am I right @jukakoski?

Copy link
Contributor

Choose a reason for hiding this comment

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

@arturi I have to check. I used webcam feature as basis for this feature and in that component there is an info screen telling if some permissions are missing. At some point there was also that kind of screen in this one which was stripped off later on.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems "selectSourceTitle: 'Valitse kaappauksen kohde'" is not used as native UI in browser is taking care of selecting the source.
"selectSourceDescription: 'Salli ruudun jako, jotta tallennus on mahdollista.'," this same thing.
Also micIsOn is not in use. Actually at the moment there is no information for the user that screen capturing is capturing mic as well. It should be optional to select audio source or disable audio.
Todo list stuff?

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.

4 participants