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.

Sign up for GitHub

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

Jump to bottom

Expose HTMLMediaElement.srcObject #365

Merged
merged 9 commits into from
Aug 20, 2021
Merged

Expose HTMLMediaElement.srcObject #365

merged 9 commits into from
Aug 20, 2021

Conversation

exoego
Copy link
Contributor

@exoego exoego commented May 13, 2019 •
edited
Loading

Closes #253.

According to MDN, srcObject was able to be a MediaStream, a MediaSource, a Blob, or a File.
But as of November 2017, browsers support only MediaStream for srcObject, so srcObject should be only MediaStream in this type facade.

@exoego
Copy link
Contributor Author

exoego commented Jun 13, 2019

Travis issue may be fixed in #368

@sjrd
Copy link
Member

sjrd commented Jun 14, 2019

#368 was merged. Could you rebase this PR?

@exoego exoego force-pushed the srcObject branch 2 times, most recently from 47a1ad3 to ae50bec Compare June 14, 2019 10:44
@exoego exoego force-pushed the srcObject branch 2 times, most recently from 6356b7e to 0a15f36 Compare February 5, 2020 02:57
@exoego
Copy link
Contributor Author

exoego commented Feb 5, 2020

@sjrd
Sorry for long absense.
I rebased and fixed formatting.
All CI jobs passed except this one: https://travis-ci.org/scala-js/scala-js-dom/jobs/646237220#L245
Could you restart only this job ?

sjrd
sjrd requested changes Mar 25, 2020
View reviewed changes
Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

Thanks @exoego, and sorry for the late review. I'm trying to get down my to-do list of PR reviews at the moment.

I just have a few small suggestions, then it's good to go.

src/main/scala/org/scalajs/dom/raw/Html.scala Outdated Show resolved Hide resolved
src/main/scala/org/scalajs/dom/experimental/mediastream/MediaStream.scala Outdated Show resolved Hide resolved
src/main/scala/org/scalajs/dom/experimental/mediastream/MediaStream.scala Outdated Show resolved Hide resolved
src/main/scala/org/scalajs/dom/experimental/mediastream/MediaStream.scala Outdated Show resolved Hide resolved
@armanbilge
Copy link
Member

Also assuming an implicit approval from @sjrd ;)

I just have a few small suggestions, then it's good to go.

@exoego
Copy link
Contributor Author

exoego commented Aug 9, 2021

I am OK having this only on v2.

@armanbilge armanbilge changed the base branch from master to series/1.x August 10, 2021 01:26
@japgolly japgolly marked this pull request as draft August 12, 2021 23:54
@japgolly
Copy link
Contributor

I am OK having this only on v2.

I think there's no reason not to add this to 1.2.0 as well cos you're just adding new stuff. Our goal for 1.2.0 is to get as much in as we can without breaking bin-compat.

@exoego exoego marked this pull request as ready for review August 13, 2021 00:20
@japgolly japgolly self-requested a review August 13, 2021 00:30
japgolly
japgolly requested changes Aug 13, 2021
View reviewed changes
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
api-reports/2_12.txt Show resolved Hide resolved
@japgolly japgolly added this to the v1.2.0 milestone Aug 13, 2021
@japgolly japgolly requested a review from armanbilge August 20, 2021 00:24
@japgolly
Copy link
Contributor

@armanbilge This is ready to go, please merge if you're happy

@armanbilge armanbilge merged commit 3388dc4 into scala-js:series/1.x Aug 20, 2021
@exoego exoego deleted the srcObject branch August 20, 2021 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjrd sjrd sjrd requested changes

@japgolly japgolly japgolly requested changes

@armanbilge armanbilge armanbilge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.2.0
Development

Successfully merging this pull request may close these issues.

HTMLMediaElement should expose srcObject
4 participants
@exoego @sjrd @armanbilge @japgolly