diff --git a/iosApp/Tests/Fixtures/SettingsContract/SOURCE b/iosApp/Tests/Fixtures/SettingsContract/SOURCE index 085809e0a..18e03a266 100644 --- a/iosApp/Tests/Fixtures/SettingsContract/SOURCE +++ b/iosApp/Tests/Fixtures/SettingsContract/SOURCE @@ -1,6 +1,6 @@ repository=https://github.com/Silo-Server/silo-server path=contracts/settings/v1 -manifest_revision=9 +manifest_revision=14 fixture_version=1 Both files are byte-identical copies of the server's canonical settings @@ -14,10 +14,10 @@ vendored manifest's revision, and the generated SettingKey.revision stop agreeing, which is the whole point of carrying the pair rather than the fixture alone. -conformance.json commit=e97d4861dfd019092dd955c4378649c48c298309 -manifest.json commit=be1c7675fc8186cdecee7002e5c03a48e9820005 -copied from commit=9d037ab6071d37d3185a203600dce810c01d044d -generated with commit=9d037ab6071d37d3185a203600dce810c01d044d (cmd/settingsgen -lang swift) +conformance.json commit=86ea1a1baf9bae2ce02b17f915817b2c76aae2b9 +manifest.json commit=86ea1a1baf9bae2ce02b17f915817b2c76aae2b9 +copied from commit=86ea1a1baf9bae2ce02b17f915817b2c76aae2b9 +generated with commit=86ea1a1baf9bae2ce02b17f915817b2c76aae2b9 (cmd/settingsgen -lang swift) manifest.json is vendored whole, including the maintainer `notes` the server strips before serving /api/v1/settings/contract. Keeping it byte-identical is @@ -32,5 +32,5 @@ contract rather than by a hand-copied table. Android vendors the same pair at shared/src/commonTest/resources/settings/v1/ in silo-android; the two SOURCE -notes are meant to stay in step. Android was at revision 7 when this copy moved -to revision 9. +notes are meant to stay in step. Android moves to revision 14 in its matching +text-opacity change. diff --git a/iosApp/Tests/Fixtures/SettingsContract/conformance.json b/iosApp/Tests/Fixtures/SettingsContract/conformance.json index 0c4fd8f94..f63d46fa0 100644 --- a/iosApp/Tests/Fixtures/SettingsContract/conformance.json +++ b/iosApp/Tests/Fixtures/SettingsContract/conformance.json @@ -1,6 +1,6 @@ { "fixture_version": 1, - "manifest_revision": 9, + "manifest_revision": 14, "description": "Cross-platform conformance cases for settings resolution. Every case runs against the shipped manifest in this directory: definitions are referenced by key, never restated, so an expectation can only be satisfied by resolving the real contract. Each platform's resolver (Go in internal/settingsresolve, TypeScript in web/src/lib/settingsResolve.ts, Kotlin and Swift in the client repos) runs every case through a hand-written runner; a runner must fail on any fixture field it does not know, because schema drift in the fixture itself is drift. A case's constraint_bindings attach a constraint to a copy of a real definition so constraint semantics stay testable even while no shipped definition carries that constraint kind. In expected entries, constrained:true requires stored_value and constraint_kind to be present, and stored_value may be null to mean the authored value was JSON null.", "cases": [ { @@ -849,6 +849,7 @@ "fontSize": "large", "fontFamily": "sans-serif", "fontColor": "#ffffff", + "textOpacity": 100, "backgroundColor": "#000000", "backgroundStyle": "box", "backgroundOpacity": 75, @@ -1002,6 +1003,68 @@ "source": "default" } ] + }, + { + "name": "advisory_age_badge_is_off_until_a_profile_asks", + "description": "The advisory-age badge is per profile and off unless that profile turned it on. p1 opted in, so it resolves true from the profile; p2 stored nothing and falls to the contract default false. A profile row must never leak across profiles here, because the setting decides what a household parent sees about a child's viewing.", + "keys": ["catalog.show_advisory_age"], + "context": { "profile_id": "p2", "device_id": "d1" }, + "stored": [ + { + "key": "catalog.show_advisory_age", + "scope": "profile", + "profile_id": "p1", + "value": true + } + ], + "expected": [ + { + "key": "catalog.show_advisory_age", + "value": false, + "source": "default" + } + ] + }, + { + "name": "advisory_age_badge_resolves_from_the_opted_in_profile", + "description": "The same stored row resolves true for the profile that owns it, with source \"profile\" rather than the default.", + "keys": ["catalog.show_advisory_age"], + "context": { "profile_id": "p1", "device_id": "d1" }, + "stored": [ + { + "key": "catalog.show_advisory_age", + "scope": "profile", + "profile_id": "p1", + "value": true + } + ], + "expected": [ + { + "key": "catalog.show_advisory_age", + "value": true, + "source": "profile" + } + ] + }, + { + "name": "theme_music_is_opt_in", + "keys": ["ui.theme_music_enabled", "ui.theme_music_loop"], + "context": {"profile_id": "p1", "device_id": "browser"}, + "stored": [], + "expected": [ + {"key": "ui.theme_music_enabled", "value": false, "source": "default"}, + {"key": "ui.theme_music_loop", "value": false, "source": "default"} + ] + }, + { + "name": "theme_music_device_overrides_profile", + "keys": ["ui.theme_music_enabled"], + "context": {"profile_id": "p1", "device_id": "browser"}, + "stored": [ + {"key": "ui.theme_music_enabled", "scope": "profile", "profile_id": "p1", "value": true}, + {"key": "ui.theme_music_enabled", "scope": "profile_device", "profile_id": "p1", "device_id": "browser", "value": false} + ], + "expected": [{"key": "ui.theme_music_enabled", "value": false, "source": "profile_device"}] } ] } diff --git a/iosApp/Tests/Fixtures/SettingsContract/manifest.json b/iosApp/Tests/Fixtures/SettingsContract/manifest.json index 3d6dd2224..427ab6964 100644 --- a/iosApp/Tests/Fixtures/SettingsContract/manifest.json +++ b/iosApp/Tests/Fixtures/SettingsContract/manifest.json @@ -1,6 +1,6 @@ { "api_version": 1, - "revision": 9, + "revision": 14, "option_sets": { "playback_audio_languages": { "type": "language_tag", @@ -365,7 +365,7 @@ "recommended_control": "select", "suggested_options": "playback_audio_languages", "unset_label": "No preference", - "notes": "Migrates user_profiles.language as the roaming fallback. Existing user_device_settings values become real overrides, and the per-series value comes from AudioPreference.audio_language. AudioPreference.audio_track_index and track_signature stay specialized: they identify a concrete track, not a default. The legacy string-only endpoint has no way to send null, so it spells \"no preference\" as the empty string and both Android and web send that to clear the choice; its validator accepts \"\" and otherwise requires a well-formed tag via settingscontract.NormalizeLanguageTag. Migration maps \"\" to no stored row, the same way playback.subtitle_mode handles it." + "notes": "Migrates user_profiles.language as the roaming fallback. Existing user_device_settings values become real overrides, and the per-series value comes from AudioPreference.audio_language. AudioPreference.audio_track_index and track_signature stay specialized: they identify a concrete track, not a default. The legacy string-only endpoint has no way to send null, so it spells \"no preference\" as the empty string and both Android and web send that to clear the choice; its validator accepts \"\" and otherwise requires a well-formed tag via settingscontract.NormalizeLanguageTag. Migration maps \"\" to no stored row, the same way playback.subtitle_mode handles it. From revision 9, the private-use tag x-silo-original means play each item in its original language: the server resolves it against the item's original_language (falling back to the profile choice when that is unknown), as it already did for the legacy \"original\" value. It is a valid value of the existing language_tag schema, so this is additive; clients that do not recognize it should label it \"Original language\" rather than as a language. The Jellyfin-compatible API maps Jellyfin's AudioLanguagePreference \"OriginalLanguage\" to it." }, { "key": "playback.subtitle_language", @@ -450,6 +450,7 @@ "backgroundColor": "#000000", "backgroundStyle": "box", "backgroundOpacity": 75, + "textOpacity": 100, "textOutline": false, "textOutlineColor": "#000000", "position": "bottom" @@ -458,7 +459,7 @@ "label": "Subtitle appearance", "description": "How subtitles are drawn during playback.", "recommended_control": "panel", - "notes": "Renamed from the unprefixed legacy key \"subtitle_appearance\". Every other canonical key carries a domain prefix, and preserving accidental key names is an explicit non-goal of the design. The rename touches three URL paths in internal/api/router.go, the admin device-settings routes, and the key constant in every client, so it cannot land without them. Migration copies the account-level legacy fallback to every existing profile and leaves device overrides unchanged, rewriting the key on each row. Revision 6 changes the fallback background from shadow to a 75% opaque box, matching the previous intended web default and the current Apple fallback. Connected clients receive the complete effective default from the server; clients should adopt revision 6 so their local and sparse-object decoding fallbacks match it. Stored profile and device overrides remain authoritative." + "notes": "Renamed from the unprefixed legacy key \"subtitle_appearance\". Every other canonical key carries a domain prefix, and preserving accidental key names is an explicit non-goal of the design. The rename touches three URL paths in internal/api/router.go, the admin device-settings routes, and the key constant in every client, so it cannot land without them. Migration copies the account-level legacy fallback to every existing profile and leaves device overrides unchanged, rewriting the key on each row. Revision 6 changes the fallback background from shadow to a 75% opaque box, matching the previous intended web default and the current Apple fallback. Connected clients receive the complete effective default from the server; clients should adopt revision 6 so their local and sparse-object decoding fallbacks match it. Stored profile and device overrides remain authoritative. Revision 14 adds the optional textOpacity property (1-100, default 100) controlling the opacity of the subtitle text itself, independent of backgroundOpacity; a value stored without it resolves to fully opaque text, matching pre-14 rendering." }, { "key": "playback.preferred_quality", @@ -645,6 +646,20 @@ "recommended_control": "panel", "notes": "Keys are canonical catalog original_language codes. Values are target BCP 47 language tags; x-silo-original means retain that source language. This key is separate from catalog.metadata_language so existing clients can continue changing the fallback without rewriting or discarding exceptions." }, + { + "key": "catalog.show_advisory_age", + "introduced_in": 10, + "persistence": "remote", + "allowed_scopes": ["profile"], + "resolution_order": ["profile", "default"], + "value_schema": { "type": "boolean" }, + "default_value": false, + "category": "catalog", + "label": "Show advisory age", + "description": "Show a recommended minimum viewer age from an advisory service, such as Common Sense Media, on item detail.", + "recommended_control": "switch", + "notes": "Profile scope because the reader is a household parent deciding what a child may watch, and they browse on their own profile. Default false: the advisory is extra chrome most viewers do not want, and only the profiles that ask for it should carry it. This setting only controls whether the badge is shown: turning it on or off never changes what a profile is allowed to watch. Restricting a profile by advisory age is a separate access field, the profile's max_advisory_age, which a household manager sets on the profile itself. Coverage is partial by nature: the provider that supplies the advisory is rate limited per day, so on a large library some titles carry one and others do not." + }, { "key": "player.hdr_enabled", "introduced_in": 1, @@ -836,7 +851,8 @@ "label": "Theme", "description": "Colour theme for the Silo interface.", "recommended_control": "select", - "notes": "Renamed from the unregistered legacy key \"ui_theme\", which the extension bag accepted without validation. Moved from account to profile scope: appearance is per household member, and the account row is copied to every profile during migration. Carries a device override because the right theme is partly a function of the screen and the room — a light theme on a phone in daylight, a dark one on a TV at night — which is the same reasoning that gives ui.text_scale one. Note that ui.custom_theme_vars and ui.custom_css stay profile-wide, so a profile's custom styling still applies on top of a device's theme override. Adding a theme is an additive enum widening. The admin-set default theme stays in server_settings and is not a user setting. Migration must also update internal/plugins/user_theme_lookup.go, which reads this value with raw SQL bound to both the old name and the account scope (SELECT value FROM user_settings WHERE user_id = $1 AND key = 'ui_theme') and feeds the X-Silo-Theme header on every plugin request. Left alone, that query matches nothing after the rename and every plugin UI silently falls back to its own theme, with no error to notice." + "deprecated": true, + "notes": "Deprecated in revision 12: the web client has one theme, Cinema Dark, which only the admin customizes (accent color plus the ui.admin_theme_vars and ui.admin_custom_css server settings), and it no longer reads or writes this key. Web-only (platforms: web). Stored values were deleted by migration 20260926233851_retire_profile_themes (SQLite user store schema v29). The definition stays published so a stale cached web bundle's write is accepted rather than failing; nothing reads what it stores. Renamed from the unregistered legacy key \"ui_theme\" at the settings cutover." }, { "key": "ui.text_scale", @@ -914,7 +930,8 @@ "label": "Custom theme variables", "description": "Per-token overrides applied on top of the selected theme.", "recommended_control": "panel", - "notes": "Renamed from the unregistered legacy key \"ui_custom_theme_vars\", which stored arbitrary unvalidated JSON." + "deprecated": true, + "notes": "Deprecated in revision 12: the web client has one theme, Cinema Dark, which only the admin customizes (accent color plus the ui.admin_theme_vars and ui.admin_custom_css server settings), and it no longer reads or writes this key. Web-only (platforms: web). Stored values were deleted by migration 20260926233851_retire_profile_themes (SQLite user store schema v29). The definition stays published so a stale cached web bundle's write is accepted rather than failing; nothing reads what it stores. Renamed from the unregistered legacy key \"ui_custom_theme_vars\", which stored arbitrary unvalidated JSON." }, { "key": "ui.custom_css", @@ -929,7 +946,8 @@ "label": "Custom CSS", "description": "Raw CSS applied on top of the selected theme.", "recommended_control": "text", - "notes": "Renamed from the unregistered legacy key \"ui_custom_css\". Sanitization stays in the web client (web/src/lib/cssSanitizer.ts); the contract only bounds length. This value is per-profile and is never applied to another profile's session." + "deprecated": true, + "notes": "Deprecated in revision 12: the web client has one theme, Cinema Dark, which only the admin customizes (accent color plus the ui.admin_theme_vars and ui.admin_custom_css server settings), and it no longer reads or writes this key. Web-only (platforms: web). Stored values were deleted by migration 20260926233851_retire_profile_themes (SQLite user store schema v29). The definition stays published so a stale cached web bundle's write is accepted rather than failing; nothing reads what it stores. Renamed from the unregistered legacy key \"ui_custom_css\". The contract only bounds length." }, { "key": "ui.date_format", @@ -974,6 +992,20 @@ "recommended_control": "select", "notes": "Moved from account to profile scope; the account row is copied to every profile during migration." }, + { + "key": "home.hide_watched_items", + "introduced_in": 12, + "persistence": "remote", + "allowed_scopes": ["profile"], + "resolution_order": ["profile", "default"], + "value_schema": { "type": "boolean" }, + "default_value": false, + "platforms": ["web", "ios", "tvos", "macos", "android", "android_tv"], + "category": "navigation", + "label": "Hide watched items from Home", + "description": "Remove watched items from ordinary Home sections while keeping Featured and watch-history sections unchanged.", + "recommended_control": "switch" + }, { "key": "ui.library_page_state", "introduced_in": 1, @@ -1043,7 +1075,7 @@ "category": "appearance", "label": "Poster badges", "description": "Which badges appear on poster cards, and where.", - "notes": "Registered from the legacy unprefixed key card_overlays, which reached the server only through the unknown-key extension bag — stored as an arbitrary string with no validation. null means the user has expressed no preference, which is what lets the server-wide admin default in the overlay-config endpoint apply; writing a resolved-but-unchosen value would silently pin them. The server-wide default configuration stays in server_settings and is not a user setting. This key describes which badges to show, not whether to show them — that is ui.card_overlays_enabled." + "notes": "Registered from the legacy unprefixed key card_overlays, which reached the server only through the unknown-key extension bag — stored as an arbitrary string with no validation. null means the user has expressed no preference, which is what lets the server-wide admin default in the overlay-config endpoint apply; writing a resolved-but-unchosen value would silently pin them. The server-wide default configuration stays in server_settings and is not a user setting. This key describes which badges to show, not whether to show them — that is ui.card_overlays_enabled. Revision 13 adds the advisory_age overlay id; a server below revision 13 rejects a value that contains it, so a client offers that badge only when manifest_revision is at least 13." }, { "key": "ui.card_overlays_enabled", @@ -1098,6 +1130,56 @@ "recommended_control": "switch", "notes": "null means the profile has not chosen whether quick actions are enabled, so the server-wide default from the overlay-config endpoint applies. An explicit profile choice overrides the server-wide default in either direction." }, + { + "key": "ui.theme_music_enabled", + "introduced_in": 11, + "persistence": "remote", + "allowed_scopes": [ + "profile", + "profile_device" + ], + "resolution_order": [ + "profile_device", + "profile", + "default" + ], + "value_schema": { + "type": "boolean" + }, + "default_value": false, + "platforms": [ + "web" + ], + "category": "playback", + "label": "Theme music", + "description": "Play local theme music while browsing movie and show detail pages.", + "recommended_control": "switch" + }, + { + "key": "ui.theme_music_loop", + "introduced_in": 11, + "persistence": "remote", + "allowed_scopes": [ + "profile", + "profile_device" + ], + "resolution_order": [ + "profile_device", + "profile", + "default" + ], + "value_schema": { + "type": "boolean" + }, + "default_value": false, + "platforms": [ + "web" + ], + "category": "playback", + "label": "Loop theme music", + "description": "Repeat theme music while the detail page stays open.", + "recommended_control": "switch" + }, { "key": "ui.next_up_mode", "introduced_in": 1, diff --git a/iosApp/Tests/PlayerSettingsFlushTests.swift b/iosApp/Tests/PlayerSettingsFlushTests.swift index 63b40a121..ac8c60391 100644 --- a/iosApp/Tests/PlayerSettingsFlushTests.swift +++ b/iosApp/Tests/PlayerSettingsFlushTests.swift @@ -1062,6 +1062,9 @@ final class PlayerSettingsFlushTests: XCTestCase { func testLegacyAppleSubtitleDefaultIsMigratedWithoutACustomOverride() async throws { let harness = try PlayerSettingsHarness() + // Production imports only after a read has reported the server's + // revision, which the subtitle-appearance write waits for. + await harness.settings.refreshFromServer() let snapshot = harness.settings.legacySnapshot() var effectiveByKey = Dictionary(uniqueKeysWithValues: snapshot.map { key, value in ( @@ -1094,6 +1097,10 @@ final class PlayerSettingsFlushTests: XCTestCase { func testSettersEncodeEachKeyAsItsContractType() async throws { let harness = try PlayerSettingsHarness() + // A real session refreshes before any settings screen is reachable. + // Without it the subtitle-appearance write waits for the server's + // manifest revision instead of being sent. + await harness.settings.refreshFromServer() harness.settings.setPreferredQuality("1080p-medium") harness.settings.setAudioLanguage("ja") @@ -1138,6 +1145,171 @@ final class PlayerSettingsFlushTests: XCTestCase { XCTAssertEqual(byKey[.playerOrientationMode]?.value, .string("rotateFreely")) } + /// `textOpacity` joined `playback.subtitle_appearance`'s schema at + /// ``SettingKey/subtitleTextOpacityRevision``, but the key has been served + /// since revision 1. An older server's schema rejects the whole object for + /// the one unknown member, so the member comes out of what is sent — and + /// only out of what is sent. + func testTextOpacityIsLeftOutOfWritesToAnOlderServer() async throws { + let harness = try PlayerSettingsHarness() + harness.transport.revision = SettingKey.subtitleTextOpacityRevision - 1 + await harness.settings.refreshFromServer() + XCTAssertFalse(harness.settings.offersSubtitleTextOpacity) + + var appearance = SubtitleAppearance.default + appearance.fontSize = .xlarge + appearance.textOpacity = 42 + await harness.settings.setSubtitleAppearance(appearance) + + let write = harness.transport.writesByKey()[.playbackSubtitleAppearance] + guard case .object(let fields) = write?.value else { + return XCTFail("expected an object write for subtitle appearance") + } + XCTAssertNil(fields["textOpacity"], "an older server must not receive textOpacity") + XCTAssertEqual(fields["fontSize"], .string("xlarge")) + XCTAssertEqual(harness.settings.subtitleAppearance.textOpacity, 42) + } + + /// Until a read for this scope reports the revision, a subtitle edit must + /// neither reach a server of unknown revision with textOpacity nor be + /// stripped for a server that stores it: it waits, keeps its value on + /// screen, and is sent whole once the refresh learns the server is current. + func testASubtitleEditWaitsForTheServerRevisionAndKeepsTextOpacity() async throws { + let harness = try PlayerSettingsHarness() + + // The first refresh for this scope has not learned its revision yet. + harness.transport.effectiveError = .transport(description: "offline") + await harness.settings.refreshFromServer() + XCTAssertNil(harness.settings.knownManifestRevision) + XCTAssertTrue(harness.settings.offersSubtitleTextOpacity, "an unknown server is not hidden from") + harness.transport.reset() + + var appearance = SubtitleAppearance.default + appearance.textOpacity = 55 + await harness.settings.setSubtitleAppearance(appearance) + XCTAssertNil( + harness.transport.writesByKey()[.playbackSubtitleAppearance], + "a write must not guess the revision of a server it has not read" + ) + + // The server still holds the profile's appearance from before the edit. + harness.transport.effective = [ + EffectiveSettingValue( + key: SettingKey.playbackSubtitleAppearance.rawValue, + value: try SettingJSONValue.encoding(SubtitleAppearance.default), + source: .scope(.profile), + scope: .profile + ), + ] + harness.transport.effectiveError = nil + await harness.settings.refreshFromServer() + + let write = harness.transport.writesByKey()[.playbackSubtitleAppearance] + guard case .object(let fields) = write?.value else { + return XCTFail("the waiting write must be sent once the revision is known") + } + XCTAssertEqual(fields["textOpacity"], .int(55)) + XCTAssertEqual(harness.settings.subtitleAppearance.textOpacity, 55, "the refresh must not revert the edit") + } + + /// A refresh that fails for the scope whose revision is already known keeps + /// that revision: going offline once must not hold every later subtitle + /// appearance write until the next successful read. + func testAFailedRefreshOfTheSameScopeKeepsTheKnownRevision() async throws { + let harness = try PlayerSettingsHarness() + await harness.settings.refreshFromServer() + XCTAssertEqual(harness.settings.knownManifestRevision, harness.transport.revision) + + harness.transport.effectiveError = .transport(description: "offline") + await harness.settings.refreshFromServer() + XCTAssertEqual(harness.settings.knownManifestRevision, harness.transport.revision) + harness.transport.reset() + + var appearance = SubtitleAppearance.default + appearance.textOpacity = 60 + await harness.settings.setSubtitleAppearance(appearance) + + guard case .object(let fields) = harness.transport.writesByKey()[.playbackSubtitleAppearance]?.value else { + return XCTFail("a known revision must not hold the write") + } + XCTAssertEqual(fields["textOpacity"], .int(60)) + } + + /// The same wait, then an older server: the waiting write is sent without + /// the member that server would reject. + func testAWaitingSubtitleEditIsSentWithoutTextOpacityToAnOlderServer() async throws { + let harness = try PlayerSettingsHarness() + harness.transport.effectiveError = .transport(description: "offline") + await harness.settings.refreshFromServer() + + var appearance = SubtitleAppearance.default + appearance.fontSize = .small + appearance.textOpacity = 77 + await harness.settings.setSubtitleAppearance(appearance) + XCTAssertNil(harness.transport.writesByKey()[.playbackSubtitleAppearance]) + + harness.transport.effectiveError = nil + harness.transport.revision = SettingKey.subtitleTextOpacityRevision - 1 + await harness.settings.refreshFromServer() + + guard case .object(let fields) = harness.transport.writesByKey()[.playbackSubtitleAppearance]?.value else { + return XCTFail("the waiting write must be sent once the revision is known") + } + XCTAssertNil(fields["textOpacity"]) + XCTAssertEqual(fields["fontSize"], .string("small")) + } + + /// The one-time legacy import goes through the same gate. On an older + /// server an unchanged appearance is not a difference just because this + /// build's encoding carries textOpacity, and a changed one is sent + /// without it. + func testLegacySubtitleImportRespectsAnOlderServersRevision() async throws { + let harness = try PlayerSettingsHarness() + let olderRevision = SettingKey.subtitleTextOpacityRevision - 1 + harness.transport.revision = olderRevision + await harness.settings.refreshFromServer() + harness.transport.reset() + + let snapshot = harness.settings.legacySnapshot() + let legacyAppearance = try XCTUnwrap(snapshot[.playbackSubtitleAppearance]) + let serverAppearance = SettingKey.playbackSubtitleAppearance.wireValue( + legacyAppearance, + forServerRevision: olderRevision + ) + XCTAssertNotEqual(serverAppearance, legacyAppearance) + func effective(_ appearance: SettingJSONValue) -> [SettingKey: EffectiveSettingValue] { + [ + .playbackSubtitleAppearance: EffectiveSettingValue( + key: SettingKey.playbackSubtitleAppearance.rawValue, + value: appearance, + source: .contractDefault + ), + ] + } + + let importedUnchanged = await harness.settings.importLegacySettingsIfNeeded( + scopeID: "older-server-unchanged", + legacySnapshot: [.playbackSubtitleAppearance: legacyAppearance], + effectiveByKey: effective(serverAppearance) + ) + XCTAssertFalse(importedUnchanged) + XCTAssertTrue(harness.transport.writes().isEmpty) + + var changed = SubtitleAppearance.default + changed.fontSize = .xxlarge + let importedChanged = await harness.settings.importLegacySettingsIfNeeded( + scopeID: "older-server-changed", + legacySnapshot: [.playbackSubtitleAppearance: try SettingJSONValue.encoding(changed)], + effectiveByKey: effective(serverAppearance) + ) + XCTAssertTrue(importedChanged) + guard case .object(let fields) = harness.transport.writesByKey()[.playbackSubtitleAppearance]?.value else { + return XCTFail("expected the legacy appearance to be imported") + } + XCTAssertNil(fields["textOpacity"], "the migration must not send an older server textOpacity") + XCTAssertEqual(fields["fontSize"], .string("xxlarge")) + } + func testNoAudioLanguagePreferenceIsSentAsJSONNull() async throws { let harness = try PlayerSettingsHarness() @@ -1192,6 +1364,7 @@ final class PlayerSettingsFlushTests: XCTestCase { func testSubtitleAppearanceIsSentAsAnObjectWithItsCamelCaseKeys() async throws { let harness = try PlayerSettingsHarness() + await harness.settings.refreshFromServer() var appearance = SubtitleAppearance.default appearance.fontSize = .xlarge @@ -1791,6 +1964,10 @@ final class FakeSettingsTransport: PlayerSettingsTransport, @unchecked Sendable /// What the next `effectiveValues` call answers with. var effective: [EffectiveSettingValue] = [] var effectiveError: SettingsAPIError? + /// The manifest revision `effectiveValues` reports. Defaults to this + /// build's head so most tests simulate a fully current server; a test of + /// revision-gated behavior overrides it to an older value. + var revision: Int = SettingKey.revision /// Artificial latency, so a test can overlap two flushes. var writeDelay: Duration? /// Models the production transport resolving the active profile after an @@ -1877,9 +2054,10 @@ final class FakeSettingsTransport: PlayerSettingsTransport, @unchecked Sendable recordedEffectiveCalls.append(keys) let settings = effective let error = effectiveError + let reportedRevision = revision lock.unlock() if let error { throw error } - return EffectiveSettingValuesResponse(settings: settings, revision: SettingKey.revision) + return EffectiveSettingValuesResponse(settings: settings, revision: reportedRevision) } func putValue(key: SettingKey, value: SettingJSONValue, profileId: String?) async throws { diff --git a/iosApp/Tests/SettingValuesAPITests.swift b/iosApp/Tests/SettingValuesAPITests.swift index bc4c9281b..f59ee3c98 100644 --- a/iosApp/Tests/SettingValuesAPITests.swift +++ b/iosApp/Tests/SettingValuesAPITests.swift @@ -415,16 +415,25 @@ final class SettingValuesAPITests: XCTestCase { /// A binding refresh must not turn every settings feature off on a server /// one revision behind: only the keys that revision lacks are gated. func testCapabilitiesFromThePreviousRevisionGateOnlyTheKeysItLacks() async throws { - SettingsStubProtocol.reset(mode: .previousContractRevision) + // The newest key isn't necessarily introduced at the head revision: a + // revision can bump for widening an existing key's schema with no new + // key of its own, as here. "One revision behind" means one behind + // whichever revision most recently introduced a key, not one behind + // head, or this test would have no gated key to check against. + let latestKeyRevision = SettingKey.allCases.map(\.introducedIn).max() ?? SettingKey.revision + let newest = SettingKey.allCases.filter { $0.introducedIn == latestKeyRevision } + XCTAssertFalse(newest.isEmpty) + + SettingsStubProtocol.reset(mode: .pinnedToRevision(latestKeyRevision - 1)) let api = await makeStubbedAPI() guard case .available(let capabilities) = await api.getContractCapabilities() else { return XCTFail("a server one revision behind must still report capabilities") } - XCTAssertEqual(capabilities.manifestRevision, SettingKey.revision - 1) + XCTAssertEqual(capabilities.manifestRevision, latestKeyRevision - 1) XCTAssertTrue(capabilities.supports(.playbackSubtitleLanguage)) XCTAssertTrue(capabilities.supports(.playbackIntroSkipMode)) - for key in SettingKey.allCases where key.introducedIn == SettingKey.revision { + for key in newest { XCTAssertFalse(capabilities.supports(key), "\(key.rawValue) is newer than the server") } } @@ -2321,14 +2330,21 @@ final class SettingValuesAPITests: XCTestCase { } func testGetEffectiveValuesFromThePreviousRevisionServesOnlyKeysItKnows() async throws { - SettingsStubProtocol.reset(mode: .previousContractRevision) + // The newest key isn't necessarily introduced at the head revision: a + // revision can bump for widening an existing key's schema (a new + // property on an object-valued setting, say) with no new key of its + // own. So "one revision behind" here means one behind whichever + // revision most recently introduced a key, not one behind head. + let latestKeyRevision = SettingKey.allCases.map(\.introducedIn).max() ?? SettingKey.revision + let newest = SettingKey.allCases.filter { $0.introducedIn == latestKeyRevision } + XCTAssertFalse(newest.isEmpty) + + SettingsStubProtocol.reset(mode: .pinnedToRevision(latestKeyRevision - 1)) let api = await makeStubbedAPI() let response = try await api.getEffectiveValues(keys: [.playbackSubtitleLanguage]) - XCTAssertEqual(response.revision, SettingKey.revision - 1) + XCTAssertEqual(response.revision, latestKeyRevision - 1) - let newest = SettingKey.allCases.filter { $0.introducedIn == SettingKey.revision } - XCTAssertFalse(newest.isEmpty) do { _ = try await api.getEffectiveValues(keys: [.playbackSubtitleLanguage] + newest) XCTFail("a resolution for keys the server does not know is only the default") @@ -2546,6 +2562,11 @@ final class SettingsStubProtocol: URLProtocol { case belowMinimumContractRevision /// A server one manifest revision behind the generated bindings. case previousContractRevision + /// A server answering at an explicit revision, for scenarios that need + /// a revision other than "one behind head" — e.g. one behind whichever + /// revision most recently introduced a key, which is not always head + /// itself: a revision can bump for a schema widening with no new key. + case pinnedToRevision(Int) /// Two expired scoped requests race one rotating account refresh. case concurrentScopedRefresh /// A scoped request owns refresh while an ordinary 401 joins it. @@ -2795,6 +2816,8 @@ final class SettingsStubProtocol: URLProtocol { responseRevision = SettingKey.minimumServerRevision - 1 case .previousContractRevision: responseRevision = SettingKey.revision - 1 + case .pinnedToRevision(let revision): + responseRevision = revision default: responseRevision = SettingKey.revision } diff --git a/iosApp/Tests/SettingsConformanceTests.swift b/iosApp/Tests/SettingsConformanceTests.swift index 20dad45d7..aebb5af53 100644 --- a/iosApp/Tests/SettingsConformanceTests.swift +++ b/iosApp/Tests/SettingsConformanceTests.swift @@ -414,6 +414,31 @@ final class SettingsConformanceTests: XCTestCase { } } + /// Object members added after their key are gated by a hand-kept revision + /// too. The manifest records that revision only in the definition's notes + /// ("Revision N adds the optional property"), so the sentence is + /// what ties the gate to the contract. + func testRevisionGatedMembersMatchTheVendoredManifest() throws { + let definitions = try rawDefinitions() + let manifestRevision = try loadManifest().revision + let gatedKeys = SettingKey.allCases.filter { !$0.revisionGatedMembers.isEmpty } + XCTAssertFalse(gatedKeys.isEmpty) + for key in gatedKeys { + let definition = try XCTUnwrap(definitions[key.rawValue], key.rawValue) + let defaults = try XCTUnwrap(definition["default_value"] as? [String: Any], key.rawValue) + let notes = try XCTUnwrap(definition["notes"] as? String, key.rawValue) + for (member, revision) in key.revisionGatedMembers { + XCTAssertNotNil(defaults[member], "\(key.rawValue) default_value has no \(member)") + XCTAssertTrue( + notes.contains("Revision \(revision) adds the optional \(member) property"), + "\(key.rawValue) notes do not say revision \(revision) added \(member)" + ) + XCTAssertGreaterThan(revision, key.introducedIn, "\(key.rawValue).\(member)") + XCTAssertLessThanOrEqual(revision, manifestRevision, "\(key.rawValue).\(member)") + } + } + } + /// A batch read fails as a whole when one of its keys is newer than the /// server, so a feature with no per-key fallback must only read keys /// every accepted server serves. The player batch carries diff --git a/iosApp/Tests/SubtitleOpacityControlsTests.swift b/iosApp/Tests/SubtitleOpacityControlsTests.swift new file mode 100644 index 000000000..9d70e5bec --- /dev/null +++ b/iosApp/Tests/SubtitleOpacityControlsTests.swift @@ -0,0 +1,56 @@ +import XCTest + +@testable import Silo + +/// The stepped opacity pickers on tvOS (player HUD and Settings) build their +/// options from ``SubtitleAppearance/opacityPickerValues(current:lowest:step:)``; +/// iOS and macOS type the value into ``PercentField``. +final class SubtitleOpacityControlsTests: XCTestCase { + + /// The HUD's text opacity picker: fully opaque must stay selectable after + /// a change, and an invisible 1% must not be offered. + func testTextOpacityStepsEndAtOneHundredAndStartVisible() { + XCTAssertEqual( + SubtitleAppearance.opacityPickerValues(current: 100, lowest: 25, step: 25), + [25, 50, 75, 100] + ) + XCTAssertEqual( + SubtitleAppearance.opacityPickerValues(current: 50, lowest: 25, step: 25), + [25, 50, 75, 100] + ) + } + + func testBackgroundOpacityStepsStartAtOff() { + XCTAssertEqual( + SubtitleAppearance.opacityPickerValues(current: 75, lowest: 0, step: 25), + [0, 25, 50, 75, 100] + ) + } + + /// A value another client stored between the steps is offered in order, + /// so the picker can select it rather than overwrite it. + func testAnOffStepCurrentValueIsKeptInOrder() { + XCTAssertEqual( + SubtitleAppearance.opacityPickerValues(current: 42, lowest: 25, step: 25), + [25, 42, 50, 75, 100] + ) + XCTAssertEqual( + SubtitleAppearance.opacityPickerValues(current: 3, lowest: 25, step: 25), + [3, 25, 50, 75, 100] + ) + XCTAssertEqual( + SubtitleAppearance.opacityPickerValues(current: 42, lowest: 5, step: 5).filter { $0 >= 40 && $0 <= 45 }, + [40, 42, 45] + ) + } + + /// macOS has no number pad, so a typed "%" or stray space must not throw + /// the edit away. + func testPercentFieldAcceptsATrailingPercentSignAndWhitespace() { + XCTAssertEqual(PercentField.parse("42"), 42) + XCTAssertEqual(PercentField.parse(" 42% "), 42) + XCTAssertEqual(PercentField.parse("42 %"), 42) + XCTAssertNil(PercentField.parse("%")) + XCTAssertNil(PercentField.parse("forty")) + } +} diff --git a/iosApp/iosApp/Components/PercentField.swift b/iosApp/iosApp/Components/PercentField.swift new file mode 100644 index 000000000..625d67710 --- /dev/null +++ b/iosApp/iosApp/Components/PercentField.swift @@ -0,0 +1,105 @@ +import SwiftUI + +/// A typed percentage value (min...100). A slider makes the low end — where a +/// few percent is the difference between legible and invisible — fiddly to +/// hit; typing the number directly doesn't have that problem. +/// +/// Shared by the Settings subtitle-appearance screen (iOS + macOS) and the +/// in-player subtitle-appearance sheet (iOS) so the two never drift on the +/// clamp, commit, or accessibility behavior. +struct PercentField: View { + let label: String + let accessibilityLabelText: String + let min: Int + let value: Int + let onCommit: (Int) -> Void + + @State private var draft: String = "" + /// True once the user has typed since the last commit or resync. Only a + /// typed draft is committed: onSubmit, focus loss and onDisappear can all + /// fire for one edit, and the later ones must neither repeat the write nor + /// put an untouched draft back over a value synced in the meantime. + @State private var isDirty = false + @FocusState private var focused: Bool + + var body: some View { + HStack(spacing: 12) { + Text(label) + Spacer() + TextField("", text: Binding( + get: { draft }, + set: { newDraft in + draft = newDraft + isDirty = true + } + )) + #if os(iOS) + .keyboardType(.numberPad) + #endif + .multilineTextAlignment(.trailing) + .frame(width: 52) + .focused($focused) + .onSubmit(commit) + .onChange(of: focused) { _, isFocused in + if !isFocused { commit() } + } + #if os(iOS) + // The number pad has no return key, so without this there is + // no way to finish an edit short of tapping elsewhere. + .toolbar { + if focused { + ToolbarItemGroup(placement: .keyboard) { + Spacer() + Button("Done") { + commit() + focused = false + } + } + } + } + #endif + Text("%") + .foregroundStyle(Color.siloSecondaryText) + } + .onAppear { draft = String(value) } + .onChange(of: value) { _, newValue in + // A synced value replaces the draft unless the user is typing. + if !focused || !isDirty { + draft = String(newValue) + isDirty = false + } + } + // Dismissing the sheet or navigating away while this field is still + // focused (swipe-away, back navigation) never fires onSubmit or the + // focus-change commit above — it just tears the view down with a + // typed-but-uncommitted draft. onDisappear is the safety net. + .onDisappear { commit() } + .accessibilityElement(children: .combine) + .accessibilityLabel(accessibilityLabelText) + .accessibilityValue("\(value) percent") + } + + private func commit() { + guard isDirty else { return } + isDirty = false + guard let parsed = Self.parse(draft) else { + draft = String(value) + return + } + let clamped = Swift.min(100, Swift.max(min, parsed)) + draft = String(clamped) + if clamped != value { + onCommit(clamped) + } + } + + /// Accepts what a user plausibly types into a percent field: surrounding + /// whitespace and a trailing "%" (macOS has no number pad to prevent it). + static func parse(_ text: String) -> Int? { + var trimmed = text.trimmingCharacters(in: .whitespacesAndNewlines) + if trimmed.hasSuffix("%") { + trimmed = String(trimmed.dropLast()).trimmingCharacters(in: .whitespaces) + } + return Int(trimmed) + } +} diff --git a/iosApp/iosApp/Networking/SettingKeyRevisions.swift b/iosApp/iosApp/Networking/SettingKeyRevisions.swift index 05ba04b31..9cf3f7a08 100644 --- a/iosApp/iosApp/Networking/SettingKeyRevisions.swift +++ b/iosApp/iosApp/Networking/SettingKeyRevisions.swift @@ -47,6 +47,12 @@ extension SettingKey { case .playerVideoSkipBackSeconds, .playerVideoSkipForwardSeconds, .playerAudiobookSkipBackSeconds, .playerAudiobookSkipForwardSeconds: return 9 + case .catalogShowAdvisoryAge: + return 10 + case .uiThemeMusicEnabled, .uiThemeMusicLoop: + return 11 + case .homeHideWatchedItems: + return 12 case .catalogMetadataLanguage, .downloadsDefaultQuality, .downloadsKeepWatched, .downloadsWifiOnly, .navShowAudiobooks, .playbackAudioLanguage, .playbackAutoPlayNext, .playbackAutoPlayNextPreview, .playbackAutoSkipCredits, @@ -71,4 +77,35 @@ extension SettingKey { func isServed(atRevision revision: Int) -> Bool { revision >= Self.minimumServerRevision && revision >= introducedIn } + + /// The manifest revision that added `textOpacity` to + /// `playback.subtitle_appearance`'s object schema. + static let subtitleTextOpacityRevision = 14 + + /// Object members added to this key's value schema after the key itself, + /// with the revision that added each. + /// + /// ``introducedIn`` cannot gate these: the key is served, but its schema is + /// `additionalProperties: false`, so an older server rejects the *whole* + /// write for one member it does not know. The flusher removes them at send + /// time for such a server, and holds the write while the server's revision + /// is not yet known. + var revisionGatedMembers: [String: Int] { + switch self { + case .playbackSubtitleAppearance: + return ["textOpacity": Self.subtitleTextOpacityRevision] + default: + return [:] + } + } + + /// `value` as a server answering at `revision` accepts it: object members + /// newer than that revision are removed, everything else is untouched. + func wireValue(_ value: SettingJSONValue, forServerRevision revision: Int) -> SettingJSONValue { + guard case .object(var fields) = value else { return value } + for (member, introducedIn) in revisionGatedMembers where revision < introducedIn { + fields.removeValue(forKey: member) + } + return .object(fields) + } } diff --git a/iosApp/iosApp/Networking/SettingKeys.generated.swift b/iosApp/iosApp/Networking/SettingKeys.generated.swift index 35598c7df..2618eecc7 100644 --- a/iosApp/iosApp/Networking/SettingKeys.generated.swift +++ b/iosApp/iosApp/Networking/SettingKeys.generated.swift @@ -29,12 +29,16 @@ public enum SettingKey: String, CaseIterable, Sendable { case catalogMetadataLanguage = "catalog.metadata_language" /// Metadata language exceptions case catalogMetadataLanguageOverrides = "catalog.metadata_language_overrides" + /// Show advisory age + case catalogShowAdvisoryAge = "catalog.show_advisory_age" /// Download quality case downloadsDefaultQuality = "downloads.default_quality" /// Keep watched downloads case downloadsKeepWatched = "downloads.keep_watched" /// Download over Wi-Fi only case downloadsWifiOnly = "downloads.wifi_only" + /// Hide watched items from Home + case homeHideWatchedItems = "home.hide_watched_items" /// Primary menu case navPrimaryMenu = "nav.primary_menu" /// Navigation shortcuts @@ -145,17 +149,23 @@ public enum SettingKey: String, CaseIterable, Sendable { case uiTextWeight = "ui.text_weight" /// Theme case uiTheme = "ui.theme" + /// Theme music + case uiThemeMusicEnabled = "ui.theme_music_enabled" + /// Loop theme music + case uiThemeMusicLoop = "ui.theme_music_loop" /// Time format case uiTimeFormat = "ui.time_format" } public extension SettingKey { - static let revision = 9 + static let revision = 14 /// Keys the server stores. The rest never leave the device. static let remote: [SettingKey] = [ .catalogMetadataLanguage, .catalogMetadataLanguageOverrides, + .catalogShowAdvisoryAge, + .homeHideWatchedItems, .navPrimaryMenu, .navShortcuts, .playbackAudioLanguage, @@ -206,6 +216,8 @@ public extension SettingKey { .uiTextScale, .uiTextWeight, .uiTheme, + .uiThemeMusicEnabled, + .uiThemeMusicLoop, .uiTimeFormat, ] @@ -225,6 +237,9 @@ public extension SettingKey { /// at write time, so editing either would rewrite the other. static let deprecated: Set = [ .playbackAutoSkipIntro, + .uiCustomCss, + .uiCustomThemeVars, + .uiTheme, ] } diff --git a/iosApp/iosApp/Screens/Player/AetherSubtitleRenderStyle.swift b/iosApp/iosApp/Screens/Player/AetherSubtitleRenderStyle.swift index b3bf05e8e..a38008f53 100644 --- a/iosApp/iosApp/Screens/Player/AetherSubtitleRenderStyle.swift +++ b/iosApp/iosApp/Screens/Player/AetherSubtitleRenderStyle.swift @@ -76,7 +76,10 @@ struct AetherSubtitleRenderStyle: Equatable { fontFamily = .custom(appearance.fontFamily.assFontName) } foreground = RGB(hex: appearance.fontColor, fallback: .white) - foregroundOpacity = Self.fraction(appearance.fontOpacity) + // System accessibility opacity and the user's synced text opacity are + // independent axes; combine them rather than letting either replace + // the other. + foregroundOpacity = Self.fraction(appearance.fontOpacity) * Self.fraction(appearance.textOpacity) drawsBox = appearance.backgroundStyle == .box && appearance.backgroundOpacity > 0 boxColor = RGB(hex: appearance.backgroundColor, fallback: .black) boxOpacity = Self.fraction(appearance.backgroundOpacity) diff --git a/iosApp/iosApp/Screens/Player/PlayerSettings.swift b/iosApp/iosApp/Screens/Player/PlayerSettings.swift index d2deb439c..20179ce1c 100644 --- a/iosApp/iosApp/Screens/Player/PlayerSettings.swift +++ b/iosApp/iosApp/Screens/Player/PlayerSettings.swift @@ -649,6 +649,26 @@ final class PlayerSettings { subtitleSystemSelectionPreferences = SystemCaptionSelectionPreferences.current() } + /// The connected server's manifest revision, as last reported by a batched + /// effective-values read. `nil` until this scope's refresh succeeds. + /// + /// Mirrors ``PlayerSettingsFlusher/knownManifestRevision`` so settings + /// screens can observe it; the flusher's copy is the one that decides what + /// a write carries (see ``SettingKey/revisionGatedMembers``). + private(set) var knownManifestRevision: Int? + + /// The settings scope ``knownManifestRevision`` was read for ("" when no + /// scope was active), or nil when no revision is known. + private var manifestRevisionScopeID: String? + + /// Whether to offer the subtitle text opacity control. Hidden only when the + /// server is known to predate the member: such a server never receives it, + /// so the choice would not survive the next refresh. + var offersSubtitleTextOpacity: Bool { + guard let knownManifestRevision else { return true } + return knownManifestRevision >= SettingKey.subtitleTextOpacityRevision + } + /// Pull every synced setting from the server and adopt it. /// /// One batched call: the server resolves all seventeen keys in a single @@ -657,6 +677,20 @@ final class PlayerSettings { @discardableResult @MainActor func refreshFromServer() async -> RefreshResult { + // This instance survives server and profile switches, so a revision + // confirmed for the previous scope must not leak into the next one. + // Until this scope's read records its own, a write whose payload + // depends on the revision waits in the queue rather than being sent + // with a guess — including through the pending-writes flush below. + // The same scope keeps its revision, so a refresh that fails offline + // does not hold every later subtitle appearance write. + let refreshScopeID = Self.currentScopeIdentifier ?? "" + if refreshScopeID != manifestRevisionScopeID { + flusher.forgetManifestRevision() + knownManifestRevision = nil + manifestRevisionScopeID = nil + } + // Capture the pre-contract values before applying the normalized cache // for this scope. That normalization intentionally turns a compound // legacy quality id into a bare resolution and would otherwise erase @@ -676,6 +710,12 @@ final class PlayerSettings { do { let response = try await flusher.effectiveValues(keys: SettingKey.playerDeviceSettings) + knownManifestRevision = flusher.knownManifestRevision + // The read goes to whichever scope is active when it is sent. If the + // scope changed while this refresh awaited, the revision belongs to + // that other scope, so it is not kept for this one. + let scopeUnchanged = (Self.currentScopeIdentifier ?? "") == refreshScopeID + manifestRevisionScopeID = knownManifestRevision != nil && scopeUnchanged ? refreshScopeID : nil let effectiveByKey = response.byKey applyEffectiveSettings(overlayingUnsettledValues(on: effectiveByKey)) @@ -699,6 +739,12 @@ final class PlayerSettings { markMigrationComplete(for: scopeID) } } + // A write that waited because it was made while the revision was + // unknown can go now. Only after the migration, whose "is anything + // owed" check must still see it as unsettled. + if flusher.hasRevisionGatedWrites { + await flushPendingDeviceSettings() + } return .refreshed } catch SettingsAPIError.serverUpgradeRequired { return .serverUpgradeRequired @@ -962,6 +1008,7 @@ final class PlayerSettings { func discardHeldDeviceSettingChanges() async -> Bool { do { let response = try await flusher.effectiveValues(keys: SettingKey.playerDeviceSettings) + knownManifestRevision = flusher.knownManifestRevision flusher.discardHeldChanges() applyEffectiveSettings(overlayingUnsettledValues(on: response.byKey)) return true @@ -1003,6 +1050,8 @@ final class PlayerSettings { /// stringified JSON the legacy string-only registry stored. Encoding goes /// through ``SettingJSONValue/encoding(_:)`` so the value's own camelCase /// keys (`fontSize`, `backgroundOpacity`) reach the server verbatim. + /// Members an older server does not know are removed by the flusher at + /// send time, not here (see ``SettingKey/revisionGatedMembers``). private func enqueueSubtitleAppearance(_ appearance: SubtitleAppearance) { guard let value = try? SettingJSONValue.encoding(appearance) else { // Unreachable for a struct of scalars, and dropping the write is @@ -1283,8 +1332,13 @@ final class PlayerSettings { guard let entry = effectiveByKey[key], entry.scope != .profileDevice else { continue } // Nothing to migrate when the resolved value already equals what // this device holds — typed comparison now, so `1` and `1.0` are - // not two different values the way their strings were. - if legacyValue.isSemanticallyEquivalent(to: entry.value) { + // not two different values the way their strings were. Compared as + // the server would store it: an older server's answer cannot carry + // a member it does not know, so that member is no difference. + let comparable = flusher.knownManifestRevision.map { + key.wireValue(legacyValue, forServerRevision: $0) + } ?? legacyValue + if comparable.isSemanticallyEquivalent(to: entry.value) { continue } flusher.enqueue(key, value: legacyValue) diff --git a/iosApp/iosApp/Screens/Player/PlayerSettingsFlusher.swift b/iosApp/iosApp/Screens/Player/PlayerSettingsFlusher.swift index 7fbd8ccfd..8c720fac8 100644 --- a/iosApp/iosApp/Screens/Player/PlayerSettingsFlusher.swift +++ b/iosApp/iosApp/Screens/Player/PlayerSettingsFlusher.swift @@ -359,6 +359,13 @@ final class PlayerSettingsFlusher: @unchecked Sendable { private var lifecycleObservers: [NSObjectProtocol] = [] + /// The connected server's manifest revision, from the last successful + /// ``effectiveValues(keys:)`` since ``forgetManifestRevision()``. + private var manifestRevision: Int? + /// Bumped by ``forgetManifestRevision()`` so a read that was already in + /// flight for the previous server cannot record that server's revision. + private var manifestRevisionGeneration: UInt64 = 0 + init( transport: PlayerSettingsTransport, debounce: Duration = PlayerSettingsFlusher.defaultDebounce, @@ -462,8 +469,49 @@ final class PlayerSettingsFlusher: @unchecked Sendable { /// On the flusher rather than reached for directly so ``PlayerSettings`` /// has one seam to fake in tests, and so the read and the writes cannot /// drift onto different transports. + /// + /// A successful read also records the server's manifest revision, which + /// decides at send time what a key with ``SettingKey/revisionGatedMembers`` + /// may carry. func effectiveValues(keys: [SettingKey]) async throws -> EffectiveSettingValuesResponse { - try await transport.effectiveValues(keys: keys) + let generation = lock.withLock { manifestRevisionGeneration } + let response = try await transport.effectiveValues(keys: keys) + lock.withLock { + if manifestRevisionGeneration == generation { + manifestRevision = response.revision + } + } + return response + } + + /// The manifest revision the last successful ``effectiveValues(keys:)`` + /// reported, or `nil` when none has succeeded since the last + /// ``forgetManifestRevision()``. + var knownManifestRevision: Int? { + lock.withLock { manifestRevision } + } + + /// Stop trusting the recorded revision, because the server or profile the + /// next read answers for may be a different one. Writes that depend on the + /// revision wait until a read records it again. + func forgetManifestRevision() { + lock.withLock { + manifestRevision = nil + manifestRevisionGeneration &+= 1 + } + } + + /// True while a write for a key with ``SettingKey/revisionGatedMembers`` + /// is queued and not yet attempted — typically one that waited because it + /// was made before ``knownManifestRevision`` was known. Ops on the backoff + /// schedule or held are left to their own triggers. + var hasRevisionGatedWrites: Bool { + lock.withLock { + pending.contains { key, write in + guard !write.isHeld, write.failedAttempts == 0, case .set = write.operation else { return false } + return !key.revisionGatedMembers.isEmpty + } + } } /// Re-read the journal and adopt anything it still owes. @@ -947,7 +995,19 @@ final class PlayerSettingsFlusher: @unchecked Sendable { do { switch write.operation { case .set(let value): - try await transport.putValue(key: key, value: value, profileId: write.profileId) + var wireValue = value + if !key.revisionGatedMembers.isEmpty { + // Decided here rather than at enqueue: the queued value + // keeps every member, so a write queued before the revision + // was known is neither sent to an older server with members + // it rejects nor stripped for a server that stores them. + guard let revision = knownManifestRevision else { + log(key, "server manifest revision not known yet", kept: true) + return .retryOnNextTrigger + } + wireValue = key.wireValue(value, forServerRevision: revision) + } + try await transport.putValue(key: key, value: wireValue, profileId: write.profileId) case .delete: try await transport.deleteValue(key: key, profileId: write.profileId) } diff --git a/iosApp/iosApp/Screens/Player/Sheets/PlayerSettingsSheet.swift b/iosApp/iosApp/Screens/Player/Sheets/PlayerSettingsSheet.swift index 76be8513a..b058b6cc0 100644 --- a/iosApp/iosApp/Screens/Player/Sheets/PlayerSettingsSheet.swift +++ b/iosApp/iosApp/Screens/Player/Sheets/PlayerSettingsSheet.swift @@ -70,9 +70,6 @@ struct PlayerSettingsSheet: View { #if os(iOS) @Environment(\.dismiss) private var dismiss - /// Slider position while the user is dragging the background-opacity - /// slider; committed (and saved) once the drag ends. - @State private var draftOpacity: Double? #endif var body: some View { @@ -231,7 +228,7 @@ struct PlayerSettingsSheet: View { .listRowInsets(EdgeInsets()) } footer: { if !matchesSystem && viewModel.settings.subtitleAppearance.isLowLegibilityRisk { - Text("Low contrast — dark text without a box or outline can be hard to read.") + Text("Low legibility — very transparent or dark text without a box or outline can be hard to read.") } } @@ -277,6 +274,10 @@ struct PlayerSettingsSheet: View { } } + if viewModel.settings.offersSubtitleTextOpacity { + textOpacityRow + } + Toggle("Text outline", isOn: appearanceBoolBinding(\.textOutline)) .tint(.siloSwitchOn) @@ -322,34 +323,31 @@ struct PlayerSettingsSheet: View { } private var appearanceOpacityRow: some View { - let committed = Double(viewModel.settings.subtitleAppearance.backgroundOpacity) - return HStack(spacing: 12) { - Text("Opacity") - Slider( - value: Binding( - get: { draftOpacity ?? committed }, - set: { draftOpacity = $0 } - ), - in: 0...100, - step: 5 - ) { editing in - guard !editing, let value = draftOpacity else { return } - draftOpacity = nil - var next = viewModel.settings.subtitleAppearance - let percent = Int(value) - if next.backgroundOpacity == percent { return } - next.backgroundOpacity = percent - Task { await viewModel.setSubtitleAppearance(next) } - } - .tint(.siloOnSurface) - Text("\(Int(draftOpacity ?? committed))%") - .monospacedDigit() - .foregroundStyle(.secondary) - .frame(minWidth: 44, alignment: .trailing) + PercentField( + label: "Opacity", + accessibilityLabelText: "Background Opacity", + min: 0, + value: viewModel.settings.subtitleAppearance.backgroundOpacity + ) { newValue in + var next = viewModel.settings.subtitleAppearance + if next.backgroundOpacity == newValue { return } + next.backgroundOpacity = newValue + Task { await viewModel.setSubtitleAppearance(next) } + } + } + + private var textOpacityRow: some View { + PercentField( + label: "Opacity", + accessibilityLabelText: "Text Opacity", + min: 1, + value: viewModel.settings.subtitleAppearance.textOpacity + ) { newValue in + var next = viewModel.settings.subtitleAppearance + if next.textOpacity == newValue { return } + next.textOpacity = newValue + Task { await viewModel.setSubtitleAppearance(next) } } - .accessibilityElement(children: .combine) - .accessibilityLabel("Background Opacity") - .accessibilityValue("\(Int(draftOpacity ?? committed)) percent") } /// Speed ladder for the sheet's picker. Mirrors the ladder the old @@ -654,6 +652,20 @@ struct PlayerSettingsSheet: View { } } + if viewModel.settings.offersSubtitleTextOpacity { + Picker("Text opacity", selection: appearanceIntBinding(\.textOpacity)) { + ForEach( + Self.opacityPickerValues( + current: viewModel.settings.subtitleAppearance.textOpacity, + from: 5 + ), + id: \.self + ) { value in + Text("\(value)%").tag(String(value)) + } + } + } + Toggle("Text outline", isOn: appearanceBoolBinding(\.textOutline)) .tint(.siloSwitchOn) @@ -671,7 +683,12 @@ struct PlayerSettingsSheet: View { } Picker("Background opacity", selection: appearanceIntBinding(\.backgroundOpacity)) { - ForEach(Array(stride(from: 0, through: 100, by: 5)), id: \.self) { value in + ForEach( + Self.opacityPickerValues( + current: viewModel.settings.subtitleAppearance.backgroundOpacity + ), + id: \.self + ) { value in Text("\(value)%").tag(String(value)) } } @@ -766,6 +783,16 @@ struct PlayerSettingsSheet: View { ) } + /// A value synced from another client (the iOS/macOS free-typed percent + /// field, or Android's) can land off this picker's 5-point cadence. + /// Without the current value folded in, this desktop `Picker` shows no + /// selection until the user picks a different option. + private static func opacityPickerValues(current: Int, from: Int = 0) -> [Int] { + (Array(stride(from: from, through: 100, by: 5)) + [current]) + .sorted() + .reduce(into: [Int]()) { acc, value in if acc.last != value { acc.append(value) } } + } + private func appearanceIntBinding(_ keyPath: WritableKeyPath) -> Binding { Binding( get: { String(viewModel.settings.subtitleAppearance[keyPath: keyPath]) }, diff --git a/iosApp/iosApp/Screens/Player/Subtitles/SubtitleAppearance.swift b/iosApp/iosApp/Screens/Player/Subtitles/SubtitleAppearance.swift index 680794c86..43c809321 100644 --- a/iosApp/iosApp/Screens/Player/Subtitles/SubtitleAppearance.swift +++ b/iosApp/iosApp/Screens/Player/Subtitles/SubtitleAppearance.swift @@ -221,6 +221,10 @@ struct SubtitleAppearance: Codable, Equatable { var backgroundColor: String var backgroundStyle: SubtitleBackgroundStylePreset var backgroundOpacity: Int + /// Opacity of the subtitle text itself, 1-100. Independent of + /// `backgroundOpacity` and distinct from the runtime-only `fontOpacity` + /// below: this one is a synced, server-backed user preference. + var textOpacity: Int var textOutline: Bool var textOutlineColor: String var position: SubtitlePositionPreset @@ -243,6 +247,7 @@ struct SubtitleAppearance: Codable, Equatable { backgroundColor: String, backgroundStyle: SubtitleBackgroundStylePreset, backgroundOpacity: Int, + textOpacity: Int = 100, textOutline: Bool, textOutlineColor: String, position: SubtitlePositionPreset, @@ -260,6 +265,7 @@ struct SubtitleAppearance: Codable, Equatable { self.backgroundColor = backgroundColor self.backgroundStyle = backgroundStyle self.backgroundOpacity = backgroundOpacity + self.textOpacity = textOpacity self.textOutline = textOutline self.textOutlineColor = textOutlineColor self.position = position @@ -292,6 +298,7 @@ struct SubtitleAppearance: Codable, Equatable { ("#d946ef", "Magenta"), ("#ef4444", "Red"), ("#3b82f6", "Blue"), + ("#9ca3af", "Gray"), ("#000000", "Black"), ] @@ -312,6 +319,7 @@ struct SubtitleAppearance: Codable, Equatable { case backgroundColor case backgroundStyle case backgroundOpacity + case textOpacity case textOutline case textOutlineColor case position @@ -325,6 +333,7 @@ struct SubtitleAppearance: Codable, Equatable { self.backgroundColor = try container.decodeIfPresent(String.self, forKey: .backgroundColor) ?? Self.default.backgroundColor self.backgroundStyle = try container.decodeIfPresent(SubtitleBackgroundStylePreset.self, forKey: .backgroundStyle) ?? Self.default.backgroundStyle self.backgroundOpacity = try container.decodeIfPresent(Int.self, forKey: .backgroundOpacity) ?? Self.default.backgroundOpacity + self.textOpacity = try container.decodeIfPresent(Int.self, forKey: .textOpacity) ?? Self.default.textOpacity self.textOutline = try container.decodeIfPresent(Bool.self, forKey: .textOutline) ?? Self.default.textOutline self.textOutlineColor = try container.decodeIfPresent(String.self, forKey: .textOutlineColor) ?? Self.default.textOutlineColor self.position = try container.decodeIfPresent(SubtitlePositionPreset.self, forKey: .position) ?? Self.default.position @@ -363,6 +372,7 @@ struct SubtitleAppearance: Codable, Equatable { if !Self.isValidHex(copy.textOutlineColor) { copy.textOutlineColor = Self.default.textOutlineColor } if !Self.isValidHex(copy.captionWindowColor) { copy.captionWindowColor = "#000000" } copy.backgroundOpacity = max(0, min(100, copy.backgroundOpacity)) + copy.textOpacity = max(1, min(100, copy.textOpacity)) copy.fontOpacity = max(0, min(100, copy.fontOpacity)) copy.captionWindowOpacity = max(0, min(100, copy.captionWindowOpacity)) if let scale = copy.systemRelativeFontScale { @@ -381,6 +391,21 @@ struct SubtitleAppearance: Codable, Equatable { return copy } + /// Values for a stepped opacity picker: `lowest` up to 100 in `step` + /// increments, plus `current` when another client stored a value between + /// them. Without `current`, a D-pad picker shows no selection, focus lands + /// on the first option, and Select silently overwrites the real value. + /// + /// `lowest` must be a multiple of `step` so 100 stays reachable. + static func opacityPickerValues(current: Int, lowest: Int, step: Int) -> [Int] { + var values = Array(stride(from: lowest, through: 100, by: step)) + if !values.contains(current) { + values.append(current) + values.sort() + } + return values + } + /// One-word style descriptor for summary rows ("Large · Box · Bottom"). var styleDescription: String { if backgroundStyle == .box { return "Box" } @@ -389,9 +414,14 @@ struct SubtitleAppearance: Codable, Equatable { return "Plain" } - /// True when the configuration risks unreadable text: a dark font - /// color with no box behind it and no outline around it. + /// True when the configuration risks unreadable text: a dark font color + /// with no box behind it and no outline around it, or text faint enough + /// that no background style would save it. var isLowLegibilityRisk: Bool { + // Below this, text is faint enough to risk illegibility regardless of + // what's behind it — a box or outline gives the shape contrast, not + // the fill itself back its opacity. + if textOpacity < 30 { return true } guard backgroundStyle != .box || backgroundOpacity == 0 else { return false } guard !textOutline && backgroundStyle != .outline else { return false } let trimmed = fontColor.hasPrefix("#") ? String(fontColor.dropFirst()) : fontColor diff --git a/iosApp/iosApp/Screens/Player/Subtitles/SubtitleAppearancePreview.swift b/iosApp/iosApp/Screens/Player/Subtitles/SubtitleAppearancePreview.swift index dde60c70b..ec8ec108b 100644 --- a/iosApp/iosApp/Screens/Player/Subtitles/SubtitleAppearancePreview.swift +++ b/iosApp/iosApp/Screens/Player/Subtitles/SubtitleAppearancePreview.swift @@ -84,7 +84,7 @@ struct SubtitleAppearancePreview: View { .multilineTextAlignment(.center) .foregroundStyle( Color(hex: appearance.fontColor) - .opacity(Double(appearance.fontOpacity) / 100) + .opacity(Double(appearance.fontOpacity) / 100 * Double(appearance.textOpacity) / 100) ) .shadow(color: outlineColor, radius: 0, x: outlineOffset, y: outlineOffset) .shadow(color: outlineColor, radius: 0, x: -outlineOffset, y: outlineOffset) diff --git a/iosApp/iosApp/Screens/Player/tvOS/TVPlayerInfoHUD.swift b/iosApp/iosApp/Screens/Player/tvOS/TVPlayerInfoHUD.swift index e382340ef..78736fa4e 100644 --- a/iosApp/iosApp/Screens/Player/tvOS/TVPlayerInfoHUD.swift +++ b/iosApp/iosApp/Screens/Player/tvOS/TVPlayerInfoHUD.swift @@ -1155,6 +1155,7 @@ private struct SubtitleAppearanceDialog: View { case font case size case textColor + case textOpacity case outlineToggle case outlineColor case backgroundColor @@ -1278,6 +1279,29 @@ private struct SubtitleAppearanceDialog: View { .focused($focusedField, equals: .textColor) .id(Field.textColor) + if viewModel.settings.offersSubtitleTextOpacity { + HUDSettingRow(label: "Text Opacity", value: textOpacityLabel) { + presentPicker( + for: .textOpacity, + HUDPickerPresentation( + title: "Text Opacity", + options: Self.opacityOptions( + current: viewModel.settings.subtitleAppearance.textOpacity, + lowest: 25 + ), + selection: String(viewModel.settings.subtitleAppearance.textOpacity), + onSelect: { value in + if let opacity = Int(value) { + updateAppearance { $0.textOpacity = opacity } + } + } + ) + ) + } + .focused($focusedField, equals: .textOpacity) + .id(Field.textOpacity) + } + HUDToggleRow( label: "Text outline", isOn: viewModel.settings.subtitleAppearance.textOutline @@ -1351,7 +1375,10 @@ private struct SubtitleAppearanceDialog: View { for: .opacity, HUDPickerPresentation( title: "Background Opacity", - options: Self.opacityOptions, + options: Self.opacityOptions( + current: viewModel.settings.subtitleAppearance.backgroundOpacity, + lowest: 0 + ), selection: String(viewModel.settings.subtitleAppearance.backgroundOpacity), onSelect: { value in if let opacity = Int(value) { @@ -1461,6 +1488,10 @@ private struct SubtitleAppearanceDialog: View { return "\(viewModel.settings.subtitleAppearance.backgroundOpacity)%" } + private var textOpacityLabel: String { + "\(viewModel.settings.subtitleAppearance.textOpacity)%" + } + private func label(for id: String, in options: [HUDDropdownOption]) -> String { options.first { $0.id.caseInsensitiveCompare(id) == .orderedSame }?.label ?? id } @@ -1477,8 +1508,13 @@ private struct SubtitleAppearanceDialog: View { private static let positionOptions: [HUDDropdownOption] = SubtitlePositionPreset.allCases.map { .init(id: $0.rawValue, label: $0.label) } - private static let opacityOptions: [HUDDropdownOption] = - stride(from: 0, through: 100, by: 25).map { .init(id: String($0), label: $0 == 0 ? "Off" : "\($0)%") } + /// 25-point steps from `lowest`, plus the current value when another + /// client stored one between them. Text opacity starts at 25 (the schema's + /// floor is 1, which is invisible); background opacity starts at Off. + private static func opacityOptions(current: Int, lowest: Int) -> [HUDDropdownOption] { + SubtitleAppearance.opacityPickerValues(current: current, lowest: lowest, step: 25) + .map { .init(id: String($0), label: $0 == 0 ? "Off" : "\($0)%") } + } private static let fontColorOptions: [HUDDropdownOption] = SubtitleAppearance.fontColors.map { .init(id: $0.hex, label: $0.label, colorHex: $0.hex) } diff --git a/iosApp/iosApp/Screens/Settings/SettingsViewModel.swift b/iosApp/iosApp/Screens/Settings/SettingsViewModel.swift index ce4935fd5..254d2b1db 100644 --- a/iosApp/iosApp/Screens/Settings/SettingsViewModel.swift +++ b/iosApp/iosApp/Screens/Settings/SettingsViewModel.swift @@ -75,6 +75,11 @@ final class SettingsViewModel { PlayerSettings.shared.effectiveSubtitleAppearance } + /// False only when the server is known to predate subtitle text opacity. + var offersSubtitleTextOpacity: Bool { + PlayerSettings.shared.offersSubtitleTextOpacity + } + /// Profile-scoped preferences written through the canonical settings API. let prefs = ProfilePrefsEditor() diff --git a/iosApp/iosApp/Screens/Settings/SubtitleSettingsView.swift b/iosApp/iosApp/Screens/Settings/SubtitleSettingsView.swift index 7775a422a..3d45ecd1b 100644 --- a/iosApp/iosApp/Screens/Settings/SubtitleSettingsView.swift +++ b/iosApp/iosApp/Screens/Settings/SubtitleSettingsView.swift @@ -7,9 +7,6 @@ import SwiftUI /// fallback. struct SubtitleSettingsView: View { @Bindable var viewModel: SettingsViewModel - /// Slider position while the user is dragging; committed (and saved - /// to the server) once the drag ends. - @State private var draftOpacity: Double? var body: some View { List { @@ -162,7 +159,7 @@ struct SubtitleSettingsView: View { .foregroundStyle(Color.siloSecondaryText) } footer: { if !manualEditingDisabled && viewModel.subtitleAppearance.isLowLegibilityRisk { - Text("Low contrast — dark text without a box or outline can be hard to read.") + Text("Low legibility — very transparent or dark text without a box or outline can be hard to read.") .foregroundStyle(Color.siloError) } } @@ -243,6 +240,21 @@ struct SubtitleSettingsView: View { selection: appearanceBinding(\.fontColor) ) + if viewModel.offersSubtitleTextOpacity { + PercentField( + label: "Opacity", + accessibilityLabelText: "Text Opacity", + min: 1, + value: viewModel.subtitleAppearance.textOpacity + ) { newValue in + var next = viewModel.subtitleAppearance + if next.textOpacity == newValue { return } + next.textOpacity = newValue + Task { await viewModel.setSubtitleAppearance(next) } + } + .foregroundStyle(Color.siloOnSurface) + } + Toggle("Text Outline", isOn: appearanceBinding(\.textOutline)) .foregroundStyle(Color.siloOnSurface) .tint(.siloSwitchOn) @@ -333,35 +345,18 @@ struct SubtitleSettingsView: View { } private var opacityRow: some View { - let committed = Double(viewModel.subtitleAppearance.backgroundOpacity) - return HStack(spacing: 12) { - Text("Opacity") - .foregroundStyle(Color.siloOnSurface) - Slider( - value: Binding( - get: { draftOpacity ?? committed }, - set: { draftOpacity = $0 } - ), - in: 0...100, - step: 5 - ) { editing in - guard !editing, let value = draftOpacity else { return } - draftOpacity = nil - var next = viewModel.subtitleAppearance - let percent = Int(value) - if next.backgroundOpacity == percent { return } - next.backgroundOpacity = percent - Task { await viewModel.setSubtitleAppearance(next) } - } - .tint(.siloOnSurface) - Text("\(Int(draftOpacity ?? committed))%") - .monospacedDigit() - .foregroundStyle(Color.siloSecondaryText) - .frame(minWidth: 44, alignment: .trailing) + PercentField( + label: "Opacity", + accessibilityLabelText: "Background Opacity", + min: 0, + value: viewModel.subtitleAppearance.backgroundOpacity + ) { newValue in + var next = viewModel.subtitleAppearance + if next.backgroundOpacity == newValue { return } + next.backgroundOpacity = newValue + Task { await viewModel.setSubtitleAppearance(next) } } - .accessibilityElement(children: .combine) - .accessibilityLabel("Background Opacity") - .accessibilityValue("\(Int(draftOpacity ?? committed)) percent") + .foregroundStyle(Color.siloOnSurface) } private func appearanceBinding( diff --git a/iosApp/iosApp/tvOS/Screens/Settings/TVSettingsComponents.swift b/iosApp/iosApp/tvOS/Screens/Settings/TVSettingsComponents.swift index 2dc0a736c..375a425ec 100644 --- a/iosApp/iosApp/tvOS/Screens/Settings/TVSettingsComponents.swift +++ b/iosApp/iosApp/tvOS/Screens/Settings/TVSettingsComponents.swift @@ -121,8 +121,20 @@ enum TVSettingsOptions { static let backgroundStyle: [TVSettingsOption] = SubtitleBackgroundStylePreset.selectableCases.map { .init(id: $0.rawValue, label: $0.label) } - static let backgroundOpacity: [TVSettingsOption] = - stride(from: 0, through: 100, by: 5).map { .init(id: String($0), label: "\($0)%") } + /// 5-point steps from `from`, plus the current value when another client + /// stored one between them (see ``SubtitleAppearance/opacityPickerValues(current:lowest:step:)``). + private static func opacityOptions(current: Int, from: Int) -> [TVSettingsOption] { + SubtitleAppearance.opacityPickerValues(current: current, lowest: from, step: 5) + .map { .init(id: String($0), label: "\($0)%") } + } + + static func backgroundOpacity(current: Int) -> [TVSettingsOption] { + opacityOptions(current: current, from: 0) + } + + static func textOpacity(current: Int) -> [TVSettingsOption] { + opacityOptions(current: current, from: 5) + } static let backgroundColor: [TVSettingsOption] = SubtitleAppearance.backgroundColors.map { .init(id: $0.hex, label: $0.label) } diff --git a/iosApp/iosApp/tvOS/Screens/Settings/TVSettingsView.swift b/iosApp/iosApp/tvOS/Screens/Settings/TVSettingsView.swift index a306ab29f..d7d8e165a 100644 --- a/iosApp/iosApp/tvOS/Screens/Settings/TVSettingsView.swift +++ b/iosApp/iosApp/tvOS/Screens/Settings/TVSettingsView.swift @@ -718,6 +718,7 @@ enum TVSettingsDetailFocus: Hashable { case subtitleFontSize case subtitleFontFamily case subtitleFontColor + case subtitleTextOpacity case subtitleOutlineColor case subtitleBackgroundStyle case subtitleBackgroundOpacity diff --git a/iosApp/iosApp/tvOS/Screens/Settings/TVSubtitleSettingsView.swift b/iosApp/iosApp/tvOS/Screens/Settings/TVSubtitleSettingsView.swift index b878dc0a4..bde01ab6d 100644 --- a/iosApp/iosApp/tvOS/Screens/Settings/TVSubtitleSettingsView.swift +++ b/iosApp/iosApp/tvOS/Screens/Settings/TVSubtitleSettingsView.swift @@ -107,7 +107,7 @@ struct TVSubtitleSettingsPane: View { if !viewModel.subtitleMatchesSystemAppearance && viewModel.subtitleAppearance.isLowLegibilityRisk { - TVSettingsFooter("Low contrast — dark text without a box or outline can be hard to read.") + TVSettingsFooter("Low legibility — very transparent or dark text without a box or outline can be hard to read.") } TVSettingsToggleRow( @@ -136,6 +136,17 @@ struct TVSubtitleSettingsPane: View { pickerRow("Font Color", options: TVSettingsOptions.fontColor, selection: viewModel.subtitleAppearance.fontColor.lowercased(), kind: .fontColor) + if viewModel.offersSubtitleTextOpacity { + TVSettingsPickerRow( + title: "Text Opacity", + value: "\(viewModel.subtitleAppearance.textOpacity)%" + ) { + guard viewModel.subtitleUsesDeviceAppearanceOverride else { return } + showPicker(.textOpacity) + } + .focused(detailFocus, equals: .subtitleTextOpacity) + } + TVSettingsToggleRow( title: "Text Outline", isOn: viewModel.subtitleAppearance.textOutline @@ -312,6 +323,14 @@ struct TVSubtitleSettingsPane: View { selection: appearanceStringBinding(\.fontColor), returnFocus: kind.returnFocus ) + case .textOpacity: + TVSettingsPickerRequest( + id: kind.id, + title: "Text Opacity", + options: TVSettingsOptions.textOpacity(current: viewModel.subtitleAppearance.textOpacity), + selection: textOpacityBinding, + returnFocus: kind.returnFocus + ) case .outlineColor: TVSettingsPickerRequest( id: kind.id, @@ -332,7 +351,7 @@ struct TVSubtitleSettingsPane: View { TVSettingsPickerRequest( id: kind.id, title: "Background Opacity", - options: TVSettingsOptions.backgroundOpacity, + options: TVSettingsOptions.backgroundOpacity(current: viewModel.subtitleAppearance.backgroundOpacity), selection: backgroundOpacityBinding, returnFocus: kind.returnFocus ) @@ -362,6 +381,7 @@ struct TVSubtitleSettingsPane: View { case fontSize case fontFamily case fontColor + case textOpacity case outlineColor case backgroundStyle case backgroundOpacity @@ -378,6 +398,7 @@ struct TVSubtitleSettingsPane: View { case .fontSize: .subtitleFontSize case .fontFamily: .subtitleFontFamily case .fontColor: .subtitleFontColor + case .textOpacity: .subtitleTextOpacity case .outlineColor: .subtitleOutlineColor case .backgroundStyle: .subtitleBackgroundStyle case .backgroundOpacity: .subtitleBackgroundOpacity @@ -421,6 +442,18 @@ struct TVSubtitleSettingsPane: View { ) } + private var textOpacityBinding: Binding { + Binding( + get: { String(viewModel.subtitleAppearance.textOpacity) }, + set: { value in + guard let opacity = Int(value) else { return } + var next = viewModel.subtitleAppearance + next.textOpacity = opacity + Task { await viewModel.setSubtitleAppearance(next) } + } + ) + } + private var backgroundOpacityBinding: Binding { Binding( get: { String(viewModel.subtitleAppearance.backgroundOpacity) },