-
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 17 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
21 changes: 21 additions & 0 deletions
21
app-android/src/main/java/io/github/droidkaigi/confsched2023/license/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,21 @@ | ||
package io.github.droidkaigi.confsched2023.license | ||
|
||
import kotlinx.collections.immutable.PersistentList | ||
import kotlinx.collections.immutable.persistentListOf | ||
|
||
data class License( | ||
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(), | ||
) |
90 changes: 90 additions & 0 deletions
90
...ndroid/src/main/java/io/github/droidkaigi/confsched2023/license/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.license | ||
|
||
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 | ||
|
||
const val ossLicenseDetailScreenRouteNameArgument = "name" | ||
const val ossLicenseDetailScreenRoute = "osslicense" | ||
fun NavGraphBuilder.nestedOssLicenseDetailScreen(onUpClick: () -> Unit) { | ||
composable("$ossLicenseDetailScreenRoute/{$ossLicenseDetailScreenRouteNameArgument}") { | ||
OssLicenseDetailScreen( | ||
onUpClick = onUpClick, | ||
) | ||
} | ||
} | ||
|
||
fun NavController.navigateOssLicenseDetailScreen( | ||
license: License, | ||
) { | ||
val licenseName = license.name.replace(' ', '-') | ||
navigate("$ossLicenseDetailScreenRoute/$licenseName") | ||
} | ||
|
||
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, | ||
) | ||
} | ||
} | ||
} | ||
} | ||
} |
59 changes: 59 additions & 0 deletions
59
...oid/src/main/java/io/github/droidkaigi/confsched2023/license/OssLicenseDetailViewModel.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,59 @@ | ||
package io.github.droidkaigi.confsched2023.license | ||
|
||
import androidx.lifecycle.SavedStateHandle | ||
import androidx.lifecycle.ViewModel | ||
import androidx.lifecycle.viewModelScope | ||
import dagger.hilt.android.lifecycle.HiltViewModel | ||
import io.github.droidkaigi.confsched2023.ui.buildUiState | ||
import kotlinx.collections.immutable.PersistentList | ||
import kotlinx.collections.immutable.persistentListOf | ||
import kotlinx.coroutines.flow.SharingStarted | ||
import kotlinx.coroutines.flow.StateFlow | ||
import kotlinx.coroutines.flow.stateIn | ||
import javax.inject.Inject | ||
|
||
@HiltViewModel | ||
class OssLicenseDetailViewModel @Inject constructor( | ||
private val ossLicenseRepository: OssLicenseRepository, | ||
savedStateHandle: SavedStateHandle, | ||
) : ViewModel() { | ||
|
||
private val licenseName = savedStateHandle.getStateFlow<String>( | ||
ossLicenseDetailScreenRouteNameArgument, | ||
"", | ||
) | ||
|
||
private val licenseStateFlow: StateFlow<PersistentList<License>> = | ||
ossLicenseRepository.licenseMetaData() | ||
.stateIn( | ||
scope = viewModelScope, | ||
started = SharingStarted.WhileSubscribed(5_000), | ||
initialValue = persistentListOf(), | ||
) | ||
|
||
private val licenseDetailStateFlow: StateFlow<List<String>> = | ||
ossLicenseRepository.licenseDetailData() | ||
.stateIn( | ||
scope = viewModelScope, | ||
started = SharingStarted.WhileSubscribed(5_000), | ||
initialValue = emptyList(), | ||
) | ||
|
||
internal val uiState: StateFlow<OssLicenseDetailScreenUiState> = | ||
buildUiState(licenseStateFlow, licenseDetailStateFlow) { metadata, detail -> | ||
val license = metadata.firstOrNull { it.name.replace(' ', '-') == licenseName.value } | ||
?.let { license -> | ||
val detail = kotlin.runCatching { | ||
val start = license.offset | ||
val end = license.offset + license.length | ||
detail.subList( | ||
fromIndex = start, | ||
toIndex = end, | ||
).joinToString("\n") | ||
}.getOrElse { "" } | ||
license.copy(detail = detail) | ||
} | ||
|
||
OssLicenseDetailScreenUiState(license) | ||
} | ||
} |
80 changes: 80 additions & 0 deletions
80
app-android/src/main/java/io/github/droidkaigi/confsched2023/license/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,80 @@ | ||
package io.github.droidkaigi.confsched2023.license | ||
|
||
import android.content.Context | ||
import io.github.droidkaigi.confsched2023.R | ||
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 | ||
|
||
public interface OssLicenseRepository { | ||
public fun licenseMetaData(): Flow<PersistentList<License>> | ||
|
||
public fun licenseDetailData(): Flow<List<String>> | ||
|
||
public fun refresh(): Unit | ||
} | ||
|
||
internal class OssLicenseRepositoryImpl @Inject constructor( | ||
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>> { | ||
refresh() | ||
return licenseMetaStateFlow | ||
} | ||
|
||
override fun licenseDetailData(): Flow<List<String>> { | ||
refresh() | ||
return licenseDetailStateFlow | ||
} | ||
|
||
override fun refresh() { | ||
licenseMetaStateFlow.value = readLicensesMetaFile() | ||
.toRowList() | ||
.parseToLibraryItem() | ||
.toPersistentList() | ||
|
||
licenseDetailStateFlow.value = readLicensesFile() | ||
.toRowList() | ||
} | ||
|
||
private fun readLicensesMetaFile(): BufferedSource { | ||
return context.resources.openRawResource(R.raw.third_party_license_metadata) | ||
.source() | ||
.buffer() | ||
} | ||
|
||
private fun readLicensesFile(): BufferedSource { | ||
return context.resources.openRawResource(R.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() } | ||
License(name, offset, length) | ||
} | ||
} | ||
|
||
private fun BufferedSource.toRowList(): List<String> { | ||
val list: MutableList<String> = mutableListOf() | ||
while (true) { | ||
val line = readUtf8Line() ?: break | ||
list.add(line) | ||
} | ||
return list | ||
} | ||
} |
21 changes: 21 additions & 0 deletions
21
...id/src/main/java/io/github/droidkaigi/confsched2023/license/OssLicenseRepositoryModule.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,21 @@ | ||
package io.github.droidkaigi.confsched2023.license | ||
|
||
import android.content.Context | ||
import dagger.Module | ||
import dagger.Provides | ||
import dagger.hilt.InstallIn | ||
import dagger.hilt.android.qualifiers.ApplicationContext | ||
import dagger.hilt.components.SingletonComponent | ||
import javax.inject.Singleton | ||
|
||
@Module | ||
@InstallIn(SingletonComponent::class) | ||
class OssLicenseRepositoryModule { | ||
@Provides | ||
@Singleton | ||
fun provideSponsorsRepository( | ||
@ApplicationContext context: Context, | ||
): OssLicenseRepository { | ||
return OssLicenseRepositoryImpl(context = context) | ||
} | ||
} |
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.
Could you put sources to appropriate modules?
I think
UI will be feature/about,
Repository's implementation and api will be core/data,
This(License) will be core/model.
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.
That's make sense.
OssLicenseRepositoryImpl
is still in the app module since limitation of Google's license plugin.Except that, placing to appropriate modules. 🙇