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

feat: Text font scaling customisation ability #8215

Merged
merged 9 commits into from
Mar 6, 2025

Conversation

david-hm-morgan
Copy link
Contributor

With reference to #8025, this is a partial proposal to see if this style of extensible customisation could be favourable to the shaka-player community:

Proposal for some accessibility options whereby an App builder can customise subtitle/caption size, with the potential of an App offering accessibility options for text size.

This style could be used for other text styling attributes in a similar way, but this is just the first step.

Will need to use these in UI text displayer
Use the CSS vars
- need to add/update UT
@avelad
Copy link
Member

avelad commented Mar 5, 2025

@tykus160 I think that custom variables are not supported on Tizen 3 and WebOS 3, https://caniuse.com/css-variables , can you confirm?

@tykus160
Copy link
Member

tykus160 commented Mar 5, 2025

@avelad they don't, similar on older WebOSes.

@david-hm-morgan david-hm-morgan changed the title Text font scaling and default background colour customisation ability feat - Text font scaling and default background colour customisation ability Mar 5, 2025
@david-hm-morgan david-hm-morgan changed the title feat - Text font scaling and default background colour customisation ability feat: Text font scaling and default background colour customisation ability Mar 5, 2025
@avelad
Copy link
Member

avelad commented Mar 5, 2025

@david-hm-morgan Then you need use something like https://stackoverflow.com/questions/38012009/how-to-feature-detect-for-css-custom-properties to keep the compatibility with old browsers (FYI @tykus160 )

@shaka-bot
Copy link
Collaborator

Incremental code coverage: 100.00%

@avelad
Copy link
Member

avelad commented Mar 5, 2025

@shaka-bot test ce

@david-hm-morgan david-hm-morgan marked this pull request as ready for review March 5, 2025 15:21
@david-hm-morgan david-hm-morgan changed the title feat: Text font scaling and default background colour customisation ability feat: Text font scaling customisation ability Mar 5, 2025
@shaka-bot
Copy link
Collaborator

@avelad: Lab tests started with arguments:

  • pr=8215
  • browser_filter=Tizen ChromecastUltra ChromecastGTV ChromecastStreamer ChromeAndroid

@avelad avelad added type: enhancement New feature or request component: captions/subtitles The issue involves captions or subtitles priority: P3 Useful but not urgent labels Mar 5, 2025
@avelad avelad added this to the v4.14 milestone Mar 5, 2025
@avelad
Copy link
Member

avelad commented Mar 5, 2025

@david-hm-morgan

The next tests are failing on Tizen:

UITextDisplayer
    ✗ correctly displays styles for cues [Safari 3.0 (Tizen 3.0)]
	Error: Expected $['font-size'] = '10px' to equal 'calc(10px*var(--shaka-text-font-size-scaling))'.
	    at <Jasmine>
	    at UserContext.<anonymous> (test/text/ui_text_displayer_unit.js:124:20 <- test/text/ui_text_displayer_unit.js:139:20)
	    at <Jasmine>

    ✗ correctly displays styles for nested cues [Safari 3.0 (Tizen 3.0)]
	Error: Expected $['font-size'] = '10px' to equal 'calc(10px*var(--shaka-text-font-size-scaling))'.
	    at <Jasmine>
	    at UserContext.<anonymous> (test/text/ui_text_displayer_unit.js:175:20 <- test/text/ui_text_displayer_unit.js:186:20)
	    at <Jasmine>

    ✗ correctly displays styles for cellResolution units [Safari 3.0 (Tizen 3.0)]
	Error: Expected $['font-size'] = '18px' to equal 'calc(18px*var(--shaka-text-font-size-scaling))'.
	    at <Jasmine>
	    at UserContext.<anonymous> (test/text/ui_text_displayer_unit.js:211:20 <- test/text/ui_text_displayer_unit.js:218:20)
	    at <Jasmine>

    ✗ correctly displays styles for percentages units [Safari 3.0 (Tizen 3.0)]
	Error: Expected $['font-size'] = '27px' to equal 'calc(27px*var(--shaka-text-font-size-scaling))'.
	    at <Jasmine>
	    at UserContext.<anonymous> (test/text/ui_text_displayer_unit.js:243:20 <- test/text/ui_text_displayer_unit.js:245:20)
	    at <Jasmine>

@avelad
Copy link
Member

avelad commented Mar 5, 2025

@shaka-bot test

@shaka-bot
Copy link
Collaborator

@avelad: Lab tests started with arguments:

  • pr=8215

@avelad
Copy link
Member

avelad commented Mar 6, 2025

@david-hm-morgan Can you add a brief documentation about it in docs/tutorials/text-displayer.md? Thanks. Otherwise LGTM

@avelad
Copy link
Member

avelad commented Mar 6, 2025

@david-hm-morgan I'll wait for the tests to pass and merge it.

@avelad avelad merged commit 18695c6 into shaka-project:main Mar 6, 2025
17 checks passed
@david-hm-morgan david-hm-morgan deleted the dhmm/eaa-font-scaling branch March 6, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: captions/subtitles The issue involves captions or subtitles priority: P3 Useful but not urgent type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants