Various improvements #46
Merged
Conversation
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
nikitos4683
changed the title
nikitos4683
force-pushed
the
main
branch
2 times, most recently
from
d1140f4 to
b687d19
Compare
Added: - Browse button to Open dialog for easier SGF file selection on Desktop platform - openFileDialog platform utility function with implementations for all platforms: - Desktop: Full implementation using AWT FileDialog - Android, iOS, WebAssembly: Stub implementations (not supported on these platforms) - openFileDialog expect declaration to PlatformUtils.kt Changed: - Improve layout of "Path or Content" field in Open dialog - now displays label above the input field with Browse button next to it - Update placeholder text in Open dialog TextField to be more descriptive Features: - Users can browse for SGF files using native file dialog on Desktop - Improved user experience with clearer input field layout and descriptive placeholders Files: - composeApp/src/androidMain/kotlin/org/dots/game/PlatformUtils.android.kt - composeApp/src/commonMain/kotlin/org/dots/game/PlatformUtils.kt - composeApp/src/commonMain/kotlin/org/dots/game/views/OpenDialog.kt - composeApp/src/desktopMain/kotlin/org/dots/game/PlatformUtils.desktop.kt - composeApp/src/nativeMain/kotlin/org/dots/game/PlatformUtils.native.kt - composeApp/src/wasmJsMain/kotlin/org/dots/game/PlatformUtils.wasmJs.kt
nikitos4683
changed the title
KvanTTT
requested changes
Oct 30, 2025
There was a problem hiding this comment.
I'm very appreciated for the improvements! There are some issues to resolve, but generally it can be merged.
Also, I recommend using different pull requests for different changes: it makes reviewing easier and allows merging the changes faster and independently.
| import androidx.compose.runtime.Composable | ||
| import androidx.compose.runtime.CompositionLocalProvider | ||
| import androidx.compose.runtime.compositionLocalOf | ||
| import java.util.Locale |
There was a problem hiding this comment.
It makes the app noncompilable on platforms other than JVM. It would be great to have compilable app at least for WasmJS (currently, it's possible to use a plug if WasmJS doesn't support the current localisation feature).
| val locale = when (language) { | ||
| Language.English -> Locale.ENGLISH | ||
| Language.Russian -> Locale.forLanguageTag("ru") | ||
| Language.System -> systemLocale // Use saved system default |
There was a problem hiding this comment.
What if systemLocale differs from English and Russian that's unsupported? Also, System is not a language, but a current OS language. Maybe remove it for now to make everything easier?
run.bat
Outdated
| REM Batch script to run the Dots game desktop application | ||
|
|
||
| echo Starting Dots game... | ||
| gradlew.bat :composeApp:run |
There was a problem hiding this comment.
Does it really simplify launching the app? FYI, there are commands for compiling a .jar or building an installer for different platforms (Dmg, Msi, Deb).
| add-job-summary: on-failure | ||
|
|
||
| - name: Execute JVM tests | ||
| run: ${{ runner.os == 'Windows' && '.\gradlew.bat' || './gradlew' }} :library:jvmTest --no-daemon --stacktrace |
There was a problem hiding this comment.
composeApp also has some tests: :composeApp:desktopTest. It's better to include them as well.
nikitos4683
force-pushed
the
main
branch
2 times, most recently
from
b97a6af to
95449d6
Compare
KvanTTT
reviewed
Oct 31, 2025
| @@ -0,0 +1,71 @@ | |||
| name: Build & Test | |||
There was a problem hiding this comment.
Also, it's better to add a status badge to README file for clarity.
Implements issue KvanTTT#38 - continuous testing on every PR - JVM tests run on Windows and macOS - ComposeApp desktop tests run on Windows and macOS - Gradle caching for faster builds - Test reports published as artifacts - Build status badge available
nikitos4683
force-pushed
the
main
branch
2 times, most recently
from
78ed268 to
407fc2e
Compare
This was referenced Oct 31, 2025
Implement pure Kotlin localization solution that works across all platforms including WASM. The standard ComposeResources API has compatibility issues with WASM, so this implementation uses a custom approach. Features: - Type-safe localization through Strings interface - English and Russian translations (EnglishStrings, RussianStrings) - Language selector in Settings dialog - Persistent language preference via Settings API - Localized enum labels (InitPosType, BaseMode, ConnectionDrawMode, PolygonDrawMode) - Reactive UI updates on language change - Zero external dependencies Updated components: - App.kt: Integrate LocalizationManager and localize main UI - NewGameDialog.kt: Localize all labels and enum options - OpenDialog.kt: Localize file browser dialog - SaveDialog.kt: Localize save options - UiSettingsForm.kt: Add language selector and localize settings - ViewUtils.kt: Enhance ModeConfig to support custom label providers New localization package structure: - Strings.kt: Interface defining all localizable strings - EnglishStrings.kt: English translations - RussianStrings.kt: Russian translations - Language.kt: Language enum (English, Russian) - LocalizationManager.kt: Language state management with persistence
KvanTTT
requested changes
Nov 1, 2025
Comment on lines
121
to
126
| Button( | ||
| onClick = { showFileDialog = true }, | ||
| modifier = Modifier.padding(0.dp) | ||
| ) { | ||
| Text("Browse") | ||
| } |
There was a problem hiding this comment.
I'd say it should not be visible on unsupported platforms since it has no effect there.
- Android: implement file selection via Storage Access Framework with content URI mapping - WASM: implement file selection via HTML input with virtual file system - Add unique name generation when file names conflict - Fix case-insensitive file extension checking
Added wasmJsTest execution step to GitHub Actions workflow and included WASM test reports in artifacts.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.