fix(settings): offer native and custom resolutions and FPS in the modern lists - #280
Merged
Merged
Conversation
…ern lists The modern settings read every list from the XML arrays alone, so a custom resolution typed under Advanced only ever appeared in the legacy view, and the device's native modes were missing from the modern resolution list entirely (nova#275). NovaDeviceListOptions mirrors what the legacy fragment appends at runtime: the custom resolution first, then the notch-adjusted size and every supported display mode, with the portrait and landscape pair for squarish screens, and the custom refresh rate and native rate on the FPS list, in the same string forms the legacy fragment stores. The maths is pure and unit-tested; NovaSettingDefinitions.load applies it once per load, and a Robolectric case proves a custom resolution set in preferences shows up in the modern list.
4 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
From nova#275 (second report in the thread): a custom resolution set under Settings > Advanced never appeared in the modern Client stream defaults > Resolution list, only in the legacy view. The modern settings read every list from the XML arrays alone; the legacy fragment appends the device's native modes and the custom values at runtime. So the modern list was missing both.
NovaDeviceListOptionsmirrors the legacy additions as pure functions: the custom resolution first, then the notch-adjusted real size and every supported display mode (TV hosts keep the same 4K gate), with the portrait/landscape pair for squarish screens and the same labels (Custom (2560x1600),Native (Portrait) (…),Native Full-Screen (…)); on the FPS list, the custom refresh rate (90.0, the string form the legacy fragment stores) and the rounded native rate. Existing values are never duplicated; bad input is ignored.NovaSettingDefinitions.loadreads the device once (forDevice) and applies it tolist_resolutionandlist_fps; every device read is wrapped so a missing display or preferences can never break settings load.Not changed: the legacy fragment (it keeps its own code path), and the legacy behaviour that a custom value appears after the settings screen is reopened.
Exact candidate
Commit:8dce261ec708241b18f8cb64c1c123d0d4336ae0Tree:46660f56799fb411a9709ccffe8555e30ea92f7bParent:224693913dfe45e2b29d796021ae5ab361bfe0eeBase:224693913dfe45e2b29d796021ae5ab361bfe0eeVerification
NovaDeviceListOptionsTest5/5 (custom appended, dedupe and bad input, squarish pair, native and notch labels, FPS forms)NovaSettingsDefinitionsTest19/19 including the new Robolectric case, which fails against master's loader (RED) and passes hereKotlinPreferenceScreensMigrationTest13/13,NovaSettingsUiStateTest7/7bash scripts/check-public-docs.sh,bash scripts/check-public-surface.sh,git diff --check: cleanNot covered: a device run of the modern settings screen; the full JVM suite and lint run in CI.