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

Improve subtitle positioning #551

Merged
merged 1 commit into from
May 21, 2016
Merged

Improve subtitle positioning #551

merged 1 commit into from
May 21, 2016

Conversation

feross
Copy link
Member

@feross feross commented May 20, 2016

Before this commit, we tweaked the subtitle position by modifying the
VTT file, line by line with a regex because I did not know it was
possible to use CSS for it.

But apparently there are Shadow DOM elements that we can use instead.

This new approach improves:

  • Wrapping long lines. Before, the text would go off the edge of the
    screen. Now it wraps intelligently.
  • The subtitles move up to get out of the way of the controls when
    those are visible.

Before this commit, we tweaked the subtitle position by modifying the
VTT file, line by line with a regex because I did not know it was
possible to use CSS for it.

But apparently there are Shadow DOM elements that we can use instead.

This new approach improves:

- Wrapping long lines. Before, the text would go off the edge of the
screen. Now it wraps intelligently.

- The subtitles move up to get out of the way of the controls when
those are visible.
@dcposch
Copy link
Contributor

dcposch commented May 21, 2016

Looks reasonable! That regex replace was pretty ugly

@dcposch dcposch merged commit 97170ce into master May 21, 2016
@grunjol
Copy link
Contributor

grunjol commented May 22, 2016

Really good change.
The old code was messing the position in the case the subtitle already has a position style defined in the line.

@feross feross deleted the better-subtitle-positioning branch May 23, 2016 00:02
@feross
Copy link
Member Author

feross commented May 23, 2016

Thanks guys!

mathiasvr pushed a commit to mathiasvr/webtorrent-desktop that referenced this pull request May 26, 2016
Before this commit, we tweaked the subtitle position by modifying the
VTT file, line by line with a regex because I did not know it was
possible to use CSS for it.

But apparently there are Shadow DOM elements that we can use instead.

This new approach improves:

- Wrapping long lines. Before, the text would go off the edge of the
screen. Now it wraps intelligently.

- The subtitles move up to get out of the way of the controls when
those are visible.
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants