labadserver / Adplayer

Adplayer reference implementation
https://github.com/labadserver/Adplayer/wiki
Other
28 stars 11 forks source link

Max z-index for Button-Wrapper (adp-wrapper) #110

Closed olwa closed 12 years ago

olwa commented 12 years ago

At this time, the adp-wrapper has a z-index from 99999999.

This comes to erros if someone uses e.g. lightboxes. See: http://rolloutadplayer.de/wordpress/gforums/topic/max-z-index

ingridgraefen commented 12 years ago

The party who creates the button must provide a dom element that takes the button. This dom element must be given a non-static position and an explicit z-index. The z-index of adp-wrapper is then irrelevant because it is caught within the containing dom element.

The button creating party also creates the physical ad. Thus this party knows what z-index for the dom element is required by the ad. The z-index of the dom element must be chosen as low as possible, i.e. 1 higher than the z-index of the ad itself.

If the ad does not interfere with other page elements then the button wouldn't either (as long as we believe that the increase by 1 does not cause a problem).

Question is now: Should we still stick to the 999999999 value in the css? The value would win if a party does not set position and z-index on the containing dom element. This would be improper usage of the button, thus resulting errors are normal. I would remove the z-index from .adp-wrapper.

Before you do so, please wait until I have committed my css changes. Your help for committing the changes would be very much appreciated as I have not done this before.

bmonaumann commented 12 years ago

@ingridgraefen You could send me your css changes and I will provide a pull request, if you want. Otherwise you have to fork this repository, commit your changes to your fork and send a pull request to this project.

bmonaumann commented 12 years ago

@ingridgraefen Here you can find a simple description of the Fork and Pull-Request workflow: https://github.com/sevntu-checkstyle/sevntu.checkstyle/wiki/Fork-and-Pull-Request-workflow

olwa commented 12 years ago

I think if we define that the one who creates the physical ad should set the z-index of the container, the complexity of the usage from the adplayer is a little bit more difficult.

So I think we shouldn't delete the z-index in the css. But we should set it to a lower number, eg 999.

The most popular lightbox-scripts usees (by default) a z-index between 1.000 and 10.000.

Is this ok?

ingridgraefen commented 12 years ago

There is no point in setting an arbitrary z-index. If the creator misses to defined an explicit z-index for the wrapper container, we will have all sorts of conflicts also with a value of 999, i.e. the icon shining through other expanded ads or expanded content (like navi).

I recommend no z-index. This might lead to an invisible button which is then the indicator of a mistake.