settings screen constraints

[Kimblebee]

Enable and disable default settings based on lens constraints and other selected settings.

⚡ Big changes:

• Each setting now has its own individual UiState located in SettingsUiState.
• SettingsViewModel sets each UiState based on device capabilities and other selected default settings

✏️ Note: The UI constraints currently disregard the current camera settings (i.e. settings changed in quick settings screen) and only focus on the state of the default settings

[temcguir]

Some quick comments. I may need to take another look.

[temcguir]

Why use a Set<DisabledRationale> here instead of just DisabledRationale?

[Kimblebee]

there could be multiple factors that prevent an option from being enabled

for example, lets say 60 fps is supported by the rear, but not the front lens on a device.
if stabilization is on the front camera, the 60fps would be disabled because of the stabilization mode and the selected lens.
I figured leaving all the constraints it as a set would leave flexibility to display on the UI or debug.

I also wasn't entirely sure how i should prioritize which single DisabledRationale to display when there are multiple

[temcguir]

Just keep in mind this is UI state; it shouldn't necessarily be making the decision about which is the most important to show, the ViewModel should.

I can see the need to possibly want to show multiple reasons, but it's also possible that those reasons could be collated into a single reason in the ViewModel. So for instance instead of:

Disabled(
    setOf(
        DisabledRationale("The lens doesn't support 60 fps"),
        DisabledRationale("The current stabilization mode does not allow 60 fps")
    )
)

It could be:

Disabled(
    DisabledRationale("The lens and stabilization mode do not allow for 60fps")
)

Or, the ViewModel can just decide to show one of the DisabledRationale, and then when that rationale is no longer relevant, it can update the Disabled state to show the next DisabledRationale.

[Kimblebee]

yeah i was thinking more along the lines of the latter.

I think it would be easier to test for disabled settings rationale using a constant test tag; and strings formatted to show all rationale may be incongruent with a test tag, or become too verbose if new constraints are introduced.

[temcguir]

Which option are you referring to by "the latter"?

My suggestion is to remove the setOf. If you have multiple disabled rationales, just use the first one. If that disabled rationale goes away, then the SettingsUiState will be updated with the next disabled rationale.

Currently you're always using the first disabled rationale in the set, but if there is a reason in the future to use multiple, you can add an overload that takes a set.

[Kimblebee]

woops, by latter I was referring to the first snippet

Disabled(
    setOf(
        DisabledRationale("The lens doesn't support 60 fps"),
        DisabledRationale("The current stabilization mode does not allow 60 fps")
    )
)

It makes sense to just add an overload down the line should it be necessary

[temcguir]

Which option are you referring to by "the latter"?

My suggestion is to remove the setOf. If you have multiple disabled rationales, just use the first one. If that disabled rationale goes away, then the SettingsUiState will be updated with the next disabled rationale.

Currently you're always using the first disabled rationale in the set, but if there is a reason in the future to use multiple, you can add an overload that takes a set.