-
Notifications
You must be signed in to change notification settings - Fork 206
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.
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
Improve OSS licenses screen #1018
Open
fumiya-kume
wants to merge
48
commits into
DroidKaigi:main
Choose a base branch
from
fumiya-kume:kuu/improve-oss-licenses-screen
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 30 commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
47ec25d
Add feature/licenses module
fumiya-kume 830e3b0
Merge branch 'main' into kuu/improve-oss-licenses-screen
fumiya-kume aa0e419
Add empty oss license screen
fumiya-kume f537b83
Add empty oss license ViewModel
fumiya-kume fde39dc
Combine screen and ViewModel
fumiya-kume 15c6b3b
Setup basis of license screen
fumiya-kume 810acb4
Remove feature/lincenses
fumiya-kume 2a326c1
Remove feature/licenses relational file
fumiya-kume b717b7d
Move classes
fumiya-kume 6ddec12
Add logic to load the licenses data
fumiya-kume e4e55ff
Add Scaffold for OSS licenses screen
fumiya-kume 71db515
Add UI implementation
fumiya-kume d2794b9
Add license detail implementation
fumiya-kume 54e8302
Fix code format
fumiya-kume 8cea72a
Add OssLicense screen
fumiya-kume 0294a88
Add OssDetailLicense screen
fumiya-kume bde016a
Merge branch 'main' into kuu/improve-oss-licenses-screen
fumiya-kume dc641f3
Move License to core/model module
fumiya-kume c18fb1d
Oss screen move to feature / about
fumiya-kume 1ae5174
Fix code format
fumiya-kume 753f9a7
Update DI config with using the optional binding
fumiya-kume c605ed3
Delete unnecessary Dagger module
fumiya-kume 35316a8
Rename unintentional method name for the navigation
fumiya-kume c7a3413
Remove un-necessary method
fumiya-kume be4982d
Cleanup fake class
fumiya-kume a7c4a4c
Cleanup duplicate logic for id of license
fumiya-kume 6afe1e6
Using multi language support feature for the screen title
fumiya-kume 1d0da32
Update library grouping logic
fumiya-kume 6207404
Fix code format
fumiya-kume 3793f2e
Merge branch 'main' into kuu/improve-oss-licenses-screen
fumiya-kume 2c4d86f
Rename default implementation for Repository
fumiya-kume b172de9
Update DI config for OssLicenseDataSource
fumiya-kume 5e5c12e
Refactoring DataFlow
fumiya-kume 65ffeeb
Fix logic
fumiya-kume 6e246d2
Remove duplicated libraries
fumiya-kume 723baf9
Refactoring
fumiya-kume aaad120
Remove unnecessary code
fumiya-kume 0aae559
Merge branch 'main' into kuu/improve-oss-licenses-screen
fumiya-kume 4ba7ae4
Merge branch 'main' into kuu/improve-oss-licenses-screen
fumiya-kume c0c4f4d
Fix visibility
fumiya-kume 1819113
Fix visibility
fumiya-kume 2e08ce5
Merge branch 'main' into kuu/improve-oss-licenses-screen
fumiya-kume a2e5eb3
Merge branch 'main' into kuu/improve-oss-licenses-screen
fumiya-kume 4183d96
Add screenshot testing for oss license screen
fumiya-kume d94c87e
Add screenshot testing for oss license detail screen
fumiya-kume a26cc92
revert comment out
fumiya-kume 1255554
Try to update the build variant when CI
fumiya-kume ecd9b9f
Revert variant change
fumiya-kume File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
69 changes: 69 additions & 0 deletions
69
...roid/src/main/java/io/github/droidkaigi/confsched2023/license/OssLicenseRepositoryImpl.kt
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
package io.github.droidkaigi.confsched2023.license | ||
|
||
import android.content.Context | ||
import io.github.droidkaigi.confsched2023.R.raw | ||
import io.github.droidkaigi.confsched2023.model.License | ||
import io.github.droidkaigi.confsched2023.model.OssLicenseRepository | ||
import kotlinx.collections.immutable.PersistentList | ||
import kotlinx.collections.immutable.persistentListOf | ||
import kotlinx.collections.immutable.toPersistentList | ||
import kotlinx.coroutines.flow.Flow | ||
import kotlinx.coroutines.flow.MutableStateFlow | ||
import okio.BufferedSource | ||
import okio.buffer | ||
import okio.source | ||
import javax.inject.Inject | ||
|
||
class OssLicenseRepositoryImpl @Inject constructor( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about moving the logic to DefaultOssLicenseDataSource to simplify the app's implementation? I think this is similar to an API, since both handle data parsing. Therefore, I suggest transferring the code to DefaultOssLicenseDataSource. class DefaultOssLicenseDataSource @Inject constructor(
private val context: Context,
) : OssLicenseDataSource {
override suspend fun licenseFlow(): OssLicense {
return withContext(context = Dispatchers.IO) {
val details = readLicensesFile().toRowList()
val metadata = readLicensesMetaFile().toRowList().parseToLibraryItem(
details = details,
)
val groupList = metadata.distinctBy { it.name }.groupByCategory()
.map {
OssLicenseGroup(
title = it.key,
licenses = it.value,
)
}.toPersistentList()
OssLicense(groupList)
}
}
private fun List<License>.groupByCategory(): Map<String, List<License>> {
val categoryList = listOf(
"Android Support",
"Android Datastore",
"Android ",
"Compose UI",
"Compose Material3",
"Compose ",
"AndroidX lifecycle",
"AndroidX ",
"Kotlin",
"Dagger",
"Firebase",
"Ktorfit",
"okhttp",
"ktor",
)
return groupBy { license ->
categoryList.firstOrNull {
license.name.startsWith(
prefix = it,
ignoreCase = true,
)
} ?: "etc"
}
}
private fun readLicensesMetaFile(): BufferedSource {
return context.resources.openRawResource(raw.third_party_license_metadata)
.source()
.buffer()
}
private fun readLicensesFile(): BufferedSource {
return context.resources.openRawResource(raw.third_party_licenses)
.source()
.buffer()
}
private fun List<String>.parseToLibraryItem(details: List<String>): List<License> {
return mapIndexed { index, value ->
val (position, name) = value.split(' ', limit = 2)
val (offset, length) = position.split(':').map { it.toInt() }
val id = name.replace(' ', '-')
License(
name = name,
id = id,
offset = offset,
length = length,
detail = details[index],
)
}
}
private fun BufferedSource.toRowList(): List<String> {
val list: MutableList<String> = mutableListOf()
while (true) {
val line = readUtf8Line() ?: break
list.add(line)
}
return list
}
} |
||
private val context: Context, | ||
) : OssLicenseRepository { | ||
|
||
private val licenseMetaStateFlow = | ||
MutableStateFlow<PersistentList<License>>(persistentListOf()) | ||
|
||
private val licenseDetailStateFlow = MutableStateFlow<List<String>>(emptyList()) | ||
|
||
override fun licenseMetaData(): Flow<PersistentList<License>> { | ||
licenseMetaStateFlow.value = readLicensesMetaFile() | ||
.toRowList() | ||
.parseToLibraryItem() | ||
.toPersistentList() | ||
return licenseMetaStateFlow | ||
} | ||
|
||
override fun licenseDetailData(): Flow<List<String>> { | ||
licenseDetailStateFlow.value = readLicensesFile() | ||
.toRowList() | ||
return licenseDetailStateFlow | ||
} | ||
|
||
private fun readLicensesMetaFile(): BufferedSource { | ||
return context.resources.openRawResource(raw.third_party_license_metadata) | ||
.source() | ||
.buffer() | ||
} | ||
|
||
private fun readLicensesFile(): BufferedSource { | ||
return context.resources.openRawResource(raw.third_party_licenses) | ||
.source() | ||
.buffer() | ||
} | ||
|
||
private fun List<String>.parseToLibraryItem(): List<License> { | ||
return map { | ||
val (position, name) = it.split(' ', limit = 2) | ||
val (offset, length) = position.split(':').map { it.toInt() } | ||
val id = name.replace(' ', '-') | ||
License(id = id, name = name, offset = offset, length = length) | ||
} | ||
} | ||
|
||
private fun BufferedSource.toRowList(): List<String> { | ||
val list: MutableList<String> = mutableListOf() | ||
while (true) { | ||
val line = readUtf8Line() ?: break | ||
list.add(line) | ||
} | ||
return list | ||
} | ||
} |
This file contains 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
22 changes: 22 additions & 0 deletions
22
core/model/src/commonMain/kotlin/io/github/droidkaigi/confsched2023/model/License.kt
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package io.github.droidkaigi.confsched2023.model | ||
|
||
import kotlinx.collections.immutable.PersistentList | ||
import kotlinx.collections.immutable.persistentListOf | ||
|
||
data class License( | ||
val id: String, | ||
val name: String, | ||
val offset: Int, | ||
val length: Int, | ||
val detail: String = "", | ||
) | ||
|
||
data class OssLicenseGroup( | ||
val title: String, | ||
val licenses: List<License>, | ||
val expand: Boolean = false, | ||
) | ||
|
||
data class OssLicense( | ||
val groupList: PersistentList<OssLicenseGroup> = persistentListOf(), | ||
) |
10 changes: 10 additions & 0 deletions
10
...el/src/commonMain/kotlin/io/github/droidkaigi/confsched2023/model/OssLicenseRepository.kt
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package io.github.droidkaigi.confsched2023.model | ||
|
||
import kotlinx.collections.immutable.PersistentList | ||
import kotlinx.coroutines.flow.Flow | ||
|
||
public interface OssLicenseRepository { | ||
public fun licenseMetaData(): Flow<PersistentList<License>> | ||
|
||
public fun licenseDetailData(): Flow<List<String>> | ||
} |
This file contains 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
90 changes: 90 additions & 0 deletions
90
...re/about/src/main/java/io/github/droidkaigi/confsched2023/about/OssLicenseDetailScreen.kt
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
package io.github.droidkaigi.confsched2023.about | ||
|
||
import androidx.compose.foundation.layout.Box | ||
import androidx.compose.foundation.layout.Column | ||
import androidx.compose.foundation.layout.padding | ||
import androidx.compose.foundation.rememberScrollState | ||
import androidx.compose.foundation.verticalScroll | ||
import androidx.compose.material.icons.Icons | ||
import androidx.compose.material.icons.filled.ArrowBack | ||
import androidx.compose.material3.ExperimentalMaterial3Api | ||
import androidx.compose.material3.Icon | ||
import androidx.compose.material3.IconButton | ||
import androidx.compose.material3.MaterialTheme | ||
import androidx.compose.material3.Scaffold | ||
import androidx.compose.material3.Text | ||
import androidx.compose.material3.TopAppBar | ||
import androidx.compose.runtime.Composable | ||
import androidx.compose.runtime.collectAsState | ||
import androidx.compose.runtime.getValue | ||
import androidx.compose.ui.Modifier | ||
import androidx.compose.ui.unit.dp | ||
import androidx.hilt.navigation.compose.hiltViewModel | ||
import androidx.navigation.NavController | ||
import androidx.navigation.NavGraphBuilder | ||
import androidx.navigation.compose.composable | ||
import io.github.droidkaigi.confsched2023.model.License | ||
|
||
const val ossLicenseDetailScreenRouteNameArgument = "name" | ||
const val ossLicenseDetailScreenRoute = "osslicense" | ||
fun NavGraphBuilder.ossLicenseDetailScreen(onUpClick: () -> Unit) { | ||
composable("$ossLicenseDetailScreenRoute/{$ossLicenseDetailScreenRouteNameArgument}") { | ||
OssLicenseDetailScreen( | ||
onUpClick = onUpClick, | ||
) | ||
} | ||
} | ||
|
||
fun NavController.navigateOssLicenseDetailScreen( | ||
license: License, | ||
) { | ||
navigate("$ossLicenseDetailScreenRoute/${license.id}") | ||
} | ||
|
||
data class OssLicenseDetailScreenUiState( | ||
val ossLicense: License? = null, | ||
) | ||
|
||
@OptIn(ExperimentalMaterial3Api::class) | ||
@Composable | ||
fun OssLicenseDetailScreen( | ||
modifier: Modifier = Modifier, | ||
viewModel: OssLicenseDetailViewModel = hiltViewModel<OssLicenseDetailViewModel>(), | ||
onUpClick: () -> Unit, | ||
) { | ||
val uiState by viewModel.uiState.collectAsState() | ||
|
||
Scaffold( | ||
modifier = modifier, | ||
topBar = { | ||
TopAppBar( | ||
title = { | ||
Text(text = "OSS ライセンス") | ||
}, | ||
navigationIcon = { | ||
IconButton(onClick = { onUpClick() }) { | ||
Icon( | ||
imageVector = Icons.Default.ArrowBack, | ||
contentDescription = "back button", | ||
) | ||
} | ||
}, | ||
) | ||
}, | ||
) { paddingValues -> | ||
Box(modifier = Modifier.padding(paddingValues)) { | ||
Column( | ||
modifier = Modifier | ||
.verticalScroll(rememberScrollState()) | ||
.padding(horizontal = 8.dp), | ||
) { | ||
uiState.ossLicense?.run { | ||
Text( | ||
text = this.detail, | ||
style = MaterialTheme.typography.bodyMedium, | ||
) | ||
} | ||
} | ||
} | ||
} | ||
} |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using Default〜 prefix for the concrete class 🙏