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.

Sign up for GitHub

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(SUP-43106): bring captions in front of entry title/description #788

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(SUP-43106): bring captions in front of entry title/description #788

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/assets/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
}

.playkit-subtitles {
z-index: 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

how do subtitles look in non-audio entries after the change ? do they look ok ?

Copy link
Author

@Lkaltura Lkaltura Jul 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

in non-audio entries, you don't have title/description on the media itself, so it's not the same case.
however, I did encounter this issue in both types:
image
i noticed these buttons have z-index=1 as well as the title/description's = 1.
we shall set the captions at z-index > 1 so that it'll be in front of the title/descreption. but setting it to 2 would cover the buttons as seen in the above photo.
shall we increase the buttons with 1 so that we can set the caption to 2?
also, what should be taken into account to determine the value of caption-z-index, other than the buttons?

Copy link
Author

@Lkaltura Lkaltura Jul 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this is how it would look like if :
.player .bottom-bar { z-index = 3 } (instead of 1 , 2 would also work fine)
and
.playkit-subtitles { z-index = 2 }
image

let me know if something else needs to be considered here

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, i agree with what sergey said below - we should localize the change to audio entries.
theres a variable on the global state called isAudio that should help with checking the entry type. see example here

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the changes should be applied only for audio-entries. Also, need check how it behaves with dual-screen plugin that manage caption z-index as well
https://github.com/kaltura/playkit-js-dual-screen/blob/e98dc19c8d19d8da36d5c0c9b2f07d29ad85612b/src/styles/global.scss#L1

Copy link
Author

@Lkaltura Lkaltura Jul 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

could you elaborate on the dual-screen ?
because there's no title/descreption on the media like audio entries,
i assume you're talking about the child video. what is the expected behavior with regards to the caption? it's supposed to be behind the captions still no?

Copy link
Author

Choose a reason for hiding this comment

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

either ways (z-index =1 or more) the captions would be in front of the 2 videos.

position: absolute;
top: 0;
bottom: 0;
Expand Down
Loading