Video maximum height rule is not enforced for fluid mode

[pavlexander]

Description

Fluid mode, while allowing the height to auto-adjust - completely ignores the maximum height, therefore making the video non-responsive to viewport size changes.

Steps to reproduce

1. Set the fluid: true flag, enforce max-height: 60vh rule.
2. Try resizing the viewport - max-height rule is ignored when viewport size is <60vh

Results

Expected

Video is scaled down

Actual

Video remains in original height

Additional Information

versions

videojs

7.17.0

browsers

all (Chrome, Opera)

OSes

Win 10 (all?)

Code to reproduce

<div class="container-fluid">
  <div class="row g-1">
    <div class="col-6 text-center">
      <h3>Video</h3>
    </div>
    <div class="col-6 text-center">
      <h3>Image</h3>
    </div>
  </div>
  <div class="row g-1">
    <div class="col-6">
      <div id="carouselExampleControls" class="carousel" data-bs-interval="false">
        <div class="carousel-inner">
          <div class="carousel-item active">
            <video id="my_video_1" class="video-js vjs-default-skin vjs-big-play-centered vjs-show-big-play-button-on-pause carouselitemmedia" controls preload="auto" data-setup='{ "fluid": true  }'>
              <source src="http://vjs.zencdn.net/v/oceans.mp4" type='video/mp4'>
            </video>
          </div>
          <div class="carousel-item">
            <img src="https://dummyimage.com/960x400/000/fff." class="carouselitemmedia">
          </div>
        </div>
        <button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleControls" data-bs-slide="prev">
          <span class="carousel-control-prev-icon" aria-hidden="true"></span>
          <span class="visually-hidden">Previous</span>
        </button>
        <button class="carousel-control-next" type="button" data-bs-target="#carouselExampleControls" data-bs-slide="next">
          <span class="carousel-control-next-icon" aria-hidden="true"></span>
          <span class="visually-hidden">Next</span>
        </button>
      </div>
    </div>
    <div class="col-6">
      <div id="carouselExampleControls2" class="carousel" data-bs-interval="false">
        <div class="carousel-inner">
          <div class="carousel-item">
            <video id="my_video_1" class="video-js vjs-default-skin vjs-big-play-centered vjs-show-big-play-button-on-pause carouselitemmedia" controls preload="auto" data-setup='{ "fluid": false, "fill": false  }'>
              <source src="http://vjs.zencdn.net/v/oceans.mp4" type='video/mp4'>
            </video>
          </div>
          <div class="carousel-item active">
            <img src="https://dummyimage.com/960x400/000/fff." class="carouselitemmedia">
          </div>
        </div>
        <button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleControls2" data-bs-slide="prev">
          <span class="carousel-control-prev-icon" aria-hidden="true"></span>
          <span class="visually-hidden">Previous</span>
        </button>
        <button class="carousel-control-next" type="button" data-bs-target="#carouselExampleControls2" data-bs-slide="next">
          <span class="carousel-control-next-icon" aria-hidden="true"></span>
          <span class="visually-hidden">Next</span>
        </button>
      </div>
    </div>
  </div>
</div>

body: {
  height: 100%;
  width: 100%;
}

#column {
  height: 100vh;
  width: 40vw;
  background-color: grey;
}

.carousel {
  width: 100%;
  background-color: red;
}

.carousel-item {
  text-align: center;
}

.carouselitemmedia {
  max-width: 100%;
  max-height: 60vh;
}

.video-js {
  background: content-box;
}

Fiddle: http://jsfiddle.net/w5v7f6ut/1/

When vh > 60 (OK)

I can manage to achieve the expected result by disabling the fluidicity (fluid: false), but then when the vh > 60 - video is no longer fitting the content size:

If the result is expected - could you tell if it's possible to achieve a mix of fit-fluid mode somehow?

[video-archivist-bot]

Hey! We've detected some video files in a comment on this issue. If you'd like to permanently archive these videos and tie them to this project, a maintainer of the project can reply to this issue with the following commands:

• for http://vjs.zencdn.net/v/oceans.mp4: say @video-archivist-bot save rEm17E

[welcome]

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.
To help make it easier for us to investigate your issue, please follow the contributing guidelines.

[mister-ben]

Fluid uses 100% width and 0 height, and padding to maintain the aspect ratio. max-height will have no effect as the height is already 0. Fit is 100% width and height. You're probably best with styles on the player's parent element to set its height and using fill on the player.

[pavlexander]

@mister-ben thank you. I was digging in the wrong direction. With your advice I was able to solve the problem (by randomly applying styles) :)

Here is the result:

so2.mp4

In case if/when video is removed here is the fiddle: http://jsfiddle.net/26xuf47j/1/

styles:

body: {
  height: 100%;
  width: 100%;
}

#column {
  height: 100vh;
  width: 40vw;
  background-color: grey;
}

.carousel {
  background-color: red;

  width: 100%;
  height: 100%;
  max-height: 60vh;
}

.carousel-inner {
  width: 100%;
  height: 100%;
}

.carousel-item {
  text-align: center;

  width: 100%;
  height: 100%;
  max-width: 100%;
}

img {
  max-width: 100%;
  max-height: 100%;
}

.video-js {
  background: content-box;
  position: static;
  height: auto;
}

are there any consequences that I need to be aware of when using static display type on videojs? It looks like the solution is working for now. I will close the ticket in the following days. Want to make sure that everything is done correctly so if anybody has any improvement suggestions your are welcomed. Would be nice if this kind of functionality would be built into videojs or documented somewhere..

[video-archivist-bot]

Hey! We've detected some video files in a comment on this issue. If you'd like to permanently archive these videos and tie them to this project, a maintainer of the project can reply to this issue with the following commands:

• for https://user-images.githubusercontent.com/20644772/149791791-ae758d8d-9d43-4b7c-badf-236efc3c64eb.mp4: say @video-archivist-bot save nwROrA

[pavlexander]

got to say that the need to specify 100 height in every single parent really messes up with my more complex layout.. Hack on top of hack to make UI responsive.

[mister-ben]

If you don't need to care about internet explorer and can use aspect-ratio, then this would work.

.video-js {
  height: auto;
  aspect-ratio: 96/40;
}

Otherwise aspect ratio has always been a hack in CSS. We could probably make the built in fluid mode work with this in future but it's a breaking change.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.