videojs default skin with vertical volumeslider causes viewport overflow in rtl layout

hartman commented 2 years ago

Description

The viewport overflows because of the use of left:-3000em within a rtl html layout

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue. Using the default skin. Set:

    controlBar: {
        volumePanel: {
            vertical: true,
            inline: false
        }
    }

Open a videojs player within a <html dir="rtl">-layout

Results

Expected

The viewport should not horizontally overflow

Actual

The viewport overflows on the left to -3000 em, as the overflow of viewport in rtl is always on the left.

This is a common problem with trying to push text offscreen. See also:

A quick way around this is using top:-negative (though might still have performance problems) or to clip the surface to invisible.

Additional Information

Please include any additional information necessary here. Including the following:

versions

videojs

what version of videojs does this occur with?

browsers

what browser are affected?

OSes

what platforms (operating systems and devices) are affected?

plugins

are any videojs plugins being used on the page? If so, please list them below.

hartman commented 2 years ago

I'm kinda wondering why that -3000 is there anyway... Its clearly to make animations work, but I can't find a situation where this does something that the opacity doesn't already handle. I'm probably overlooking the real reason it is this value, but for now on en.wp I'm going to simply set it to -3.5em; which it is also when visible.

gkatsev commented 2 years ago

I believe it's moved away because otherwise, you could still interact with the control even when it wasn't visible, which we didn't want. There is probably a way to fix this with any writing direction layout. I guess the simplest is to add something like:

html[dir=rtl] .video-js .vjs-volume-panel .vjs-volume-control.vjs-volume-vertical {
  right: -3000em;
}

Though, ideally, we'd find something that works well for both.

lalitkumawat1m commented 1 year ago

I want to work on this issue could you please assign it to me

mister-ben commented 1 year ago

Feel free, thanks for contributing.

evoxf1 commented 1 year ago

is anyone working on this issue

evoxf1 commented 1 year ago

all good first issues are taken smh :(

amtins commented 1 year ago

@evoxf1 I've just created a draft PR, I still have to test it before I can submit it or see if there isn't a better approach.

videojs / video.js