Skip to content

Add in-memory theme picker in Settings #15

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.

Sign up for GitHub

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

Merged
TheRealAshik merged 2 commits into from
May 16, 2026
Merged

Add in-memory theme picker in Settings #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c640ab
feat(settings): add in-memory theme picker
TheRealAshik May 16, 2026
a7acd0e
Merge remote-tracking branch 'origin/develop' into feature/settings-t…
TheRealAshik May 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion composeApp/src/commonMain/kotlin/dev/therealashik/github/App.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.darkColorScheme
import androidx.compose.material3.lightColorScheme
import androidx.compose.runtime.Composable
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.navigation.compose.NavHost
import androidx.navigation.compose.composable
import androidx.navigation.compose.rememberNavController
Expand All @@ -20,13 +22,21 @@ import dev.therealashik.github.settings.AddPatScreen
import dev.therealashik.github.settings.CodeOptionsScreen
import dev.therealashik.github.settings.NotificationOptionsScreen
import dev.therealashik.github.settings.SettingsScreen
import dev.therealashik.github.settings.ThemePreference
import dev.therealashik.github.settings.ThemeStateHolder

private val LightColorScheme = lightColorScheme()
private val DarkColorScheme = darkColorScheme()

@Composable
fun App() {
val colorScheme = if (isSystemInDarkTheme()) DarkColorScheme else LightColorScheme
val themePreference by ThemeStateHolder.themePreference.collectAsState()

val colorScheme = when (themePreference) {
ThemePreference.SYSTEM -> if (isSystemInDarkTheme()) DarkColorScheme else LightColorScheme
ThemePreference.LIGHT -> LightColorScheme
ThemePreference.DARK -> DarkColorScheme
}

MaterialTheme(colorScheme = colorScheme) {
val navController = rememberNavController()
Expand Down
49 changes: 48 additions & 1 deletion composeApp/src/commonMain/kotlin/dev/therealashik/github/settings/SettingsScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ fun SettingsScreen(
viewModel: SettingsViewModel = viewModel { SettingsViewModel() }
) {
val uiState by viewModel.uiState.collectAsState()
val themePreference by viewModel.themePreference.collectAsState()
var showAccountsSheet by remember { mutableStateOf(false) }
var showThemeDialog by remember { mutableStateOf(false) }

// Refresh token state when returning to this screen
val lifecycle = LocalLifecycleOwner.current.lifecycle
Expand Down Expand Up @@ -92,7 +94,7 @@ fun SettingsScreen(
SettingsRow(
title = stringResource(Res.string.settings_theme),
subtitle = uiState.themeValue,
onClick = { /* TODO */ }
onClick = { showThemeDialog = true }
)
SettingsRow(
title = stringResource(Res.string.settings_code_options),
Expand Down Expand Up @@ -159,6 +161,51 @@ fun SettingsScreen(
}
}
}

if (showThemeDialog) {
AlertDialog(
onDismissRequest = { showThemeDialog = false },
title = { Text(text = stringResource(Res.string.settings_theme)) },
text = {
Column {
ThemePreference.values().forEach { preference ->
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 16, 2026

Choose a reason for hiding this comment

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

medium

Use entries instead of values() for Enum classes. In Kotlin 1.9.0+, entries is preferred as it returns a pre-allocated immutable list, avoiding the array allocation overhead of values() on every call.

Suggested change
ThemePreference.values().forEach { preference ->
ThemePreference.entries.forEach { preference ->

Row(
Modifier
.fillMaxWidth()
.clickable {
viewModel.setTheme(preference)
showThemeDialog = false
}
.padding(vertical = 12.dp),
verticalAlignment = Alignment.CenterVertically
) {
RadioButton(
selected = preference == themePreference,
onClick = {
viewModel.setTheme(preference)
showThemeDialog = false
}
)
Spacer(modifier = Modifier.width(8.dp))
Text(
text = when (preference) {
ThemePreference.SYSTEM -> stringResource(Res.string.settings_follow_system)
ThemePreference.LIGHT -> "Light"
ThemePreference.DARK -> "Dark"
Comment on lines +193 to +194
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 16, 2026

Choose a reason for hiding this comment

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

medium

Hardcoded strings "Light" and "Dark" should be replaced with localized string resources to support internationalization, ensuring consistency with other localized strings like settings_follow_system.

Suggested change
ThemePreference.LIGHT -> "Light"
ThemePreference.DARK -> "Dark"
ThemePreference.LIGHT -> stringResource(Res.string.settings_theme_light)
ThemePreference.DARK -> stringResource(Res.string.settings_theme_dark)

},
style = MaterialTheme.typography.bodyLarge
)
}
}
}
},
confirmButton = {
TextButton(onClick = { showThemeDialog = false }) {
Text(stringResource(Res.string.content_description_close))
}
}
)
}
}

@Composable
Expand Down
17 changes: 17 additions & 0 deletions composeApp/src/commonMain/kotlin/dev/therealashik/github/settings/SettingsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,28 @@ class SettingsViewModel : ViewModel() {
private val _uiState = MutableStateFlow(SettingsUiState())
val uiState: StateFlow<SettingsUiState> = _uiState.asStateFlow()

val themePreference: StateFlow<ThemePreference> = ThemeStateHolder.themePreference.asStateFlow()

private val _signOutEvent = MutableSharedFlow<Unit>()
val signOutEvent: SharedFlow<Unit> = _signOutEvent.asSharedFlow()

init {
refresh()

viewModelScope.launch {
themePreference.collect { pref ->
val displayString = when (pref) {
ThemePreference.SYSTEM -> "Follow system"
ThemePreference.LIGHT -> "Light"
ThemePreference.DARK -> "Dark"
}
_uiState.value = _uiState.value.copy(themeValue = displayString)
Comment on lines +32 to +37
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 16, 2026

Choose a reason for hiding this comment

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

medium

This mapping logic duplicates the string resolution found in the UI and uses hardcoded English strings. Additionally, since this is set in the init block, it might not update if the system language changes while the app is running. It is recommended to move display string resolution to the UI layer or use a more robust localization strategy for ViewModels.

}
}
}

fun setTheme(preference: ThemePreference) {
ThemeStateHolder.themePreference.value = preference
}

fun refresh() {
Expand Down
13 changes: 13 additions & 0 deletions composeApp/src/commonMain/kotlin/dev/therealashik/github/settings/ThemePreference.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package dev.therealashik.github.settings

import kotlinx.coroutines.flow.MutableStateFlow

enum class ThemePreference {
SYSTEM,
LIGHT,
DARK
}

object ThemeStateHolder {
val themePreference = MutableStateFlow(ThemePreference.SYSTEM)
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 16, 2026

Choose a reason for hiding this comment

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

medium

Exposing a MutableStateFlow directly in a public object breaks encapsulation. It's better to expose a read-only StateFlow and provide a dedicated method to update the value, ensuring that state changes are controlled and predictable.

}
Loading