separation of (bg)color from other defaults #3713
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TheModMaker
reviewed
Oct 25, 2021
| @@ -303,12 +303,15 @@ | |||
| transition-delay: 500ms; | |||
|
|
|||
| /* These are defaults which are overridden by JS or cue styles. */ | |||
| span:not(.shaka-text-wrapper, .shaka-text-wrapper span) { | |||
There was a problem hiding this comment.
This selector will match nothing since the second not group will match any <span> elements. So this is effectively removing those defaults. I'll take a look at this issue since I'm working on a similar bug.
joeyparrish
pushed a commit
that referenced
this pull request
Jan 5, 2022
First, the positioning of cues was incorrect. We need to explicitly set all the position attributes when we position elements with the "absolute" position. Second, if we position a text <span> manually, the background will fill the whole region. So to keep the background wrapping the text, we need to add another <span> for the text. Third, the background and font color should not be set on every <span> element since it won't allow parent cues to set the inherited value. So this moves the defaults to the top-level text element and allows parent cue elements to override this. It also would make app CSS easier to override. Because background color isn't inherited through CSS, the default is set in JavaScript instead. Fixes #3521 Fixes #3600 Closes #3713 Backported to v3.1.x Change-Id: I45fc88dcac4a0a062e1474087f24c80d98eef619
joeyparrish
pushed a commit
that referenced
this pull request
Jan 5, 2022
First, the positioning of cues was incorrect. We need to explicitly set all the position attributes when we position elements with the "absolute" position. Second, if we position a text <span> manually, the background will fill the whole region. So to keep the background wrapping the text, we need to add another <span> for the text. Third, the background and font color should not be set on every <span> element since it won't allow parent cues to set the inherited value. So this moves the defaults to the top-level text element and allows parent cue elements to override this. It also would make app CSS easier to override. Because background color isn't inherited through CSS, the default is set in JavaScript instead. Fixes #3521 Fixes #3600 Closes #3713 Backported to v3.2.x Change-Id: I45fc88dcac4a0a062e1474087f24c80d98eef619
github-actions
bot
added
the
status: archived
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Solution to loss of (bg)color for nested captions via simpler CSS (
.lessfile) update:shaka-text-wrapperclassed spans)shaka-text-wrapperclassed spans OR descendents ofshaka-text-wrapperclassed spans.Fixes XML text subtitles (bg)color not applied to doubly nested cues #3521
Screenshots (optional)
Type of change
not work as expected)
Checklist:
./build/all.pyand the build passes./build/test.pyand all tests pass