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

Updating video player CSS to work better in the new design #1883

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

jkachel
Copy link
Contributor

@jkachel jkachel commented Sep 12, 2023

What are the relevant tickets?

Closes #1880

Description (What does it do?)

The relevant VideoJS CSS was located in a set of CSS selectors that meant it wouldn't be applied on the new design product page, which broke the player. This moves those styles where they will apply, and makes some tweaks to the styles to make videos fit better in the sidebar.

This also adds a new videojs tag that can be used to drop our standard VideoJS tag, and will allow us to make global changes to this (as we'll be able to update the tag template in one place).

Screenshots (if appropriate):

YouTube embed:
image

HLS embed:
image

How can this be tested?

Set up a course with a video URL. Viewing the course should give you a play button and video controls (in some fashion - embedded ones for HLS video, YouTube ones for a YouTube video).

.. by making it actually exist there. Also includes some sizing tweaks for the new sidecard layout (especially for HLS videos).
Copy link
Contributor

@rachellougee rachellougee left a comment

Choose a reason for hiding this comment

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

Looks good. Both youtube and HLS videos are working

@jkachel jkachel merged commit 9bee98f into main Sep 12, 2023
@jkachel jkachel deleted the jkachel/1880-video-player-fix branch September 12, 2023 20:31
This was referenced Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New design: featured videos don't appear to work
2 participants