Skip to content

Conversation

materoy
Copy link

@materoy materoy commented Jan 1, 2025

This pr is a refactor of SettingsFragment to SettingsScreen in compose

Still in draft phase, almost complete, feel free to review, suggest edits and bug fixes

@CLAassistant
Copy link

CLAassistant commented Jan 1, 2025

CLA assistant check
All committers have signed the CLA.

@materoy materoy marked this pull request as draft January 1, 2025 22:02
@materoy materoy changed the title Settings screen refactor to compose refactor: SettingsFragment to compose Jan 1, 2025
@andrekir
Copy link
Member

andrekir commented Jan 3, 2025

Thanks for your contribution! As a general guideline, we want to follow the design of a traditional Settings/Preference screen, making use of our existing "Preference" components (like SwitchPreference and DropdownPreference).

The BLE/TCP/Serial device scan/selection could probably move to an item in the Settings screen to maintain a cleaner layout, but open to ideas and suggestions on how best to organize this.

Items currently in the overflow menu, like Theme and Language, will also be moved into this screen in the future.

devices.value = mutableMapOf<String, DeviceListEntry>().apply {
fun addDevice(entry: DeviceListEntry) { this[entry.fullAddress] = entry }
suspend fun addDevice(entry: DeviceListEntry) {
_uiState.emit(uiState.value.copy(devices = uiState.value.devices + (entry.fullAddress to entry)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use MutableStateFlow .update {} instead of .emit

@jamesarich
Copy link
Collaborator

@materoy this got completed as part of #1835 thanks!

@jamesarich jamesarich closed this May 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants