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

Fix positioning video elements using z-index #179

Merged
merged 1 commit into from
Jun 14, 2016

Conversation

saghul
Copy link
Collaborator

@saghul saghul commented Jun 10, 2016

Instead of adding the video element view as a su-view of the WebView,
add it as a subview of it's superview. This puts the WebView and the
video views at the same level (aka, they are siblings) and the layer's
zPosition is considered amongst siblings.

Since an ASCII diagram is worth a thousand images, this is the layout
before this patch:

+-------------------------------+
|           superview           |
+-------------------------------+
+-------------------------------+
|            WebView            |
+-------------------------------+
+--------------+ +--------------+
|  videoView1  | |  videoView2  |
+--------------+ +--------------+

And after:

+---------------------------------------+
|                superview              |
+---------------------------------------+
+---------+ +------------+ +------------+
| WebView | | videoView1 | | videoView2 |
+---------+ +------------+ +------------+

This makes it possible to order videos "behind" the WebView, which this
patch makes transparent, and thus have HTML elements on top of the video
views.

Props to @1N50MN14 for writing a first version of (what became) this patch.

Supersedes: #98

@saghul
Copy link
Collaborator Author

saghul commented Jun 10, 2016

/cc @ibc please review this when you can.

@saghul
Copy link
Collaborator Author

saghul commented Jun 14, 2016

@ibc gentle reminder :-) 🙏

self.elementView.userInteractionEnabled = false
self.elementView.hidden = true
self.elementView.backgroundColor = UIColor.blackColor()
self.elementView.backgroundColor = UIColor.clearColor()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don't you let the video view to be black when no video is rendered (as a common video element)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought of it but was a bit undecided. I can change it back. It actually looks weird if the video doesn't cover the entire surface and object-fit is not used. 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you mean with "cover the entire surface"? What does "surface" stand for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I mean that if the webview has a transparent background and the video element doesn't cover it completely, you'd see the (transparent) background, which is weird whitish. I agree it's better to have black bars on the video, or if the user wants they can set object-fit to get rid of them.

@ibc
Copy link
Collaborator

ibc commented Jun 14, 2016

It looks very nice and seems to satisfy a proper set of requirements. Please correct me if I'm wrong:

  • If no z-index is given the behavior is the same as now.
  • If two videos have the same z-index (or don't have it) and share position, last one wins.
  • Videos with negative z-index are just visible if body's background-color or background is set to transparent.
  • A video with z-index: -3 will be placed behind videos with z-index: -1, z-index: 0, z-index: 10, or unset z-index.
  • A video with z-index: 999 will be placed on top of videos with z-index: 888, z-index: 0, z-index: -10, or unset z-index.

Right?

Also, some questions made inline.

@saghul
Copy link
Collaborator Author

saghul commented Jun 14, 2016

Right?

Yes on all accounts.

Also, some questions made inline.

I'll fixup the video background color in a sec.

Instead of adding the video element view as a su-view of the WebView,
add it as a subview of it's superview. This puts the WebView and the
video views at the same level (aka, they are siblings) and the layer's
zPosition is considered amongst siblings.

Since an ASCII diagram is worth a thousand images, this is the layout
before this patch:

+-------------------------------+
|           superview           |
+-------------------------------+
+-------------------------------+
|            WebView            |
+-------------------------------+
+--------------+ +--------------+
|  videoView1  | |  videoView2  |
+--------------+ +--------------+

And after:

+---------------------------------------+
|                superview              |
+---------------------------------------+
+---------+ +------------+ +------------+
| WebView | | videoView1 | | videoView2 |
+---------+ +------------+ +------------+

This makes it possible to order videos "behind" the WebView, which this
patch makes transparent, and thus have HTML elements on top of the video
views.

Props to @1N50MN14 for writing a first version of (what became) this patch.
@saghul
Copy link
Collaborator Author

saghul commented Jun 14, 2016

Force-pushed.

@ibc ibc merged commit 01d58ce into cordova-rtc:master Jun 14, 2016
@ibc
Copy link
Collaborator

ibc commented Jun 14, 2016

Looks great. So.... let's merge it!

@saghul
Copy link
Collaborator Author

saghul commented Jun 14, 2016

Sweet, thanks for the quick review @ibc! Also thanks @1N50MN14 for the initial patch! 🎉 🍰

@ibc
Copy link
Collaborator

ibc commented Jun 14, 2016

3.0.1 is out. Thanks guys.

@RobAWarner
Copy link

Hey, I wondered if you could help me understand how to use this correctly as I am struggling to get HTML buttons over top of the video.
Currently, I have the videos in a window that pops up over the main content after a call is accepted. I'm able to get the local video over the remote video but cannot seem to get some buttons over the remote video. I've tried to follow the example app you have but wonder if its just how I have it set up.

I have a container with a z-index of 100 that's absolute positioned (and is set to 'display:none' until needed), inside a video container that's absolute positioned, inside that a local video with z-index of 110 and remote video with 105, then an absolute positioned button container with a z-index higher than the videos but is behind them. Are you able to offer any guidance? Thanks.

@saghul
Copy link
Collaborator Author

saghul commented Sep 26, 2016

@MonsterKiller you need to make the div containing the video have a higher z-index. Then make its background transparent. Effectively you need to position the browser view on top of the video view (they are different native views) and make the webview transparent so the video is seen through it, but the buttons will be painted on top.

@ibc
Copy link
Collaborator

ibc commented Sep 26, 2016

@MonsterKiller may you please start a thread in the public mailing list? Please, don't comment issues on an already merged PR.

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.

3 participants