Skip to content
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

PDF MVI 리팩토링 #29

Merged
merged 11 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
android:minSdkVersion="30" />

<application
android:name=".SeeDocs"
android:allowBackup="true"
android:dataExtractionRules="@xml/data_extraction_rules"
android:fullBackupContent="@xml/backup_rules"
Expand Down
20 changes: 17 additions & 3 deletions app/src/main/java/kr/co/seedocs/SeeDocs.kt
Original file line number Diff line number Diff line change
@@ -1,17 +1,31 @@
package kr.co.seedocs

import android.app.Application
import dagger.hilt.android.HiltAndroidApp
import kr.co.main.di.mainModule
import org.koin.android.ext.koin.androidContext
import org.koin.android.ext.koin.androidLogger
import org.koin.core.context.startKoin
import org.koin.core.logger.Level
import timber.log.Timber

@HiltAndroidApp

Choose a reason for hiding this comment

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

힐트를 제거하고 코인으로 변경한 이유가 무엇인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

KMP로의 확장성, 새로운 기술스택 추가하기 위해 Koin을 사용했습니다

Choose a reason for hiding this comment

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

  1. KMP로의 확장성이 현재 프로젝트의 목적 또는 범위에 부합한가요?
  2. 새로운 기술스택은 어떤 것이 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현재 프로젝트의 범위에 부합하진 않지만 추후에 추가적인 작업을 할 수 있다고 생각했습니다
새로운 기술스택은 Hilt가 아닌 Koin을 사용하는것을 생각했습니다

Choose a reason for hiding this comment

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

  1. 프로젝트 시작 전의 설계나 계획을 바꾸는 것은 실제 프로덕트에서는 매우 제한적인 작업입니다. 특히 기술 스택을 변경하는 것은 더욱 그렇습니다.
  2. Koin으로 개발한 경험이 기존에 없으셨었나요? 제 기억이 가물가물..
  3. 개발을 할 때에는 많이 미래지향적인 것은 바람직하지 않습니다. YAGNI 원칙을 떠올려보면 좋겠습니다.
  4. 어떠한 이유로 설계, 계획이 바뀌는 것은 기존의 것이 잘못되었음을 의미하기도 합니다. 이러한 상황이 반복되지 않도록 셀프 피드백을 해주세요.

이 코멘트는 완료하겠습니다.

class SeeDocs: Application() {
private val allModules =
listOf(
mainModule,
)

class SeeDocs : Application() {

override fun onCreate() {
super.onCreate()

if (BuildConfig.DEBUG) {
Timber.plant(Timber.DebugTree())
}

startKoin {
androidContext(this@SeeDocs)
androidLogger(if (BuildConfig.DEBUG) Level.DEBUG else Level.NONE)
modules(allModules)
}
}
}
4 changes: 0 additions & 4 deletions build-logic/convention/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,5 @@ gradlePlugin {
id = "seedocs.feature"
implementationClass = "SeeDocsFeatureConventionPlugin"
}
register("SeeDocsHilt") {
id = "seedocs.hilt"
implementationClass = "SeeDocsHiltConventionPlugin"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import kr.co.convention.implementations
import kr.co.convention.libs
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.kotlin.dsl.dependencies
Expand All @@ -9,7 +10,6 @@ class SeeDocsFeatureConventionPlugin : Plugin<Project> {
with(target) {
pluginManager.apply {
apply("seedocs.library")
apply("seedocs.hilt")
}

dependencies {
Expand All @@ -19,6 +19,7 @@ class SeeDocsFeatureConventionPlugin : Plugin<Project> {
project(":core:data"),
project(":core:navigation"),
project(":core:model"),
libs.koin.compose
)
}
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class SeeDocsLibraryConventionPlugin : Plugin<Project> {
with(pluginManager) {
apply("com.android.library")
apply("org.jetbrains.kotlin.android")
apply("com.google.devtools.ksp")
}

extensions.configure<LibraryExtension> {
Expand All @@ -31,7 +32,8 @@ class SeeDocsLibraryConventionPlugin : Plugin<Project> {

dependencies {
implementations(
libs.timber
libs.timber,
libs.koin.core
)
testImplementations(
kotlin("test")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import org.gradle.kotlin.dsl.the
internal val Project.libs get() = the<LibrariesForLibs>()

fun Project.App() {
project.pluginManager.apply {
apply("com.google.devtools.ksp")
}
project.dependencies {
implementations(
libs.androidx.core.splashscreen,
Expand All @@ -22,7 +25,7 @@ fun Project.App() {
libs.compose.activity,
libs.compose.lifecycle.runtime,
libs.compose.navigation,
libs.compose.hilt.navigation,
libs.koin.compose,
)

testImplementations(
Expand Down
1 change: 0 additions & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ plugins {
alias(libs.plugins.kotlin.serialization) apply false
alias(libs.plugins.firebase.crashlytics) apply false
alias(libs.plugins.gms) apply false
alias(libs.plugins.hilt) apply false
alias(libs.plugins.ksp) apply false
alias(libs.plugins.room) apply false
}
Expand Down
1 change: 0 additions & 1 deletion core/data/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import kr.co.convention.setNamespace

plugins {
alias(libs.plugins.seedocs.library)
alias(libs.plugins.seedocs.hilt)
}

android {
Expand Down
1 change: 0 additions & 1 deletion core/database/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import kr.co.convention.setNamespace

plugins {
alias(libs.plugins.seedocs.library)
alias(libs.plugins.seedocs.hilt)
alias(libs.plugins.kotlin.serialization)
alias(libs.plugins.room)
}
Expand Down

This file was deleted.

11 changes: 11 additions & 0 deletions feature/main/src/main/java/kr/co/main/di/MainModule.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package kr.co.main.di

import kr.co.di.pdfModule
import org.koin.dsl.module

val mainModule =
module {
includes(
pdfModule,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ internal fun MainNavHost(
padding = padding
)

pdfNavGraph(
popBackStack = navigator::popBackStack
)
pdfNavGraph()
}
}
10 changes: 10 additions & 0 deletions feature/pdf/src/main/java/kr/co/di/PdfModule.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package kr.co.di

import kr.co.pdf.PdfViewModel
import org.koin.core.module.dsl.viewModel
import org.koin.dsl.module

val pdfModule =
module {
viewModel { PdfViewModel() }
}
2 changes: 0 additions & 2 deletions feature/pdf/src/main/java/kr/co/navigation/PdfNavGraph.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import androidx.navigation.toRoute
import kr.co.pdf.PdfRoute

fun NavGraphBuilder.pdfNavGraph(
popBackStack: () -> Unit = {}
) {
composable<Route.Pdf> {
PdfRoute(
path = it.toRoute<Route.Pdf>().path,
popBackStack = popBackStack
)
}
}
68 changes: 33 additions & 35 deletions feature/pdf/src/main/java/kr/co/pdf/PdfScreen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import androidx.compose.material3.Text
import androidx.compose.material3.TopAppBar
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
Expand All @@ -43,65 +44,59 @@ import androidx.compose.ui.unit.dp
import androidx.core.text.isDigitsOnly
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import kotlinx.coroutines.launch
import kr.co.pdf.model.UiIntent
import kr.co.pdf.model.UiState
import kr.co.ui.theme.SeeDocsTheme
import kr.co.ui.theme.Theme
import kr.co.ui.util.rememberTopBarState
import kr.co.ui.widget.SimpleTextField
import kr.co.ui.widget.TextFieldInputType
import kr.co.util.PdfToBitmap
import kr.co.util.rememberPdfState
import net.engawapg.lib.zoomable.rememberZoomState
import net.engawapg.lib.zoomable.zoomable
import org.koin.androidx.compose.koinViewModel
import java.io.File

@Composable
internal fun PdfRoute(
path: String,
popBackStack: () -> Unit = {},
viewModel: PdfViewModel = koinViewModel(),
) {
val uiState by viewModel.uiState.collectAsStateWithLifecycle()

val uri = remember { Uri.fromFile(File(path)) }

val context = LocalContext.current

val scope = rememberCoroutineScope()

val renderer = remember(uri) {
LaunchedEffect(path) {
context.contentResolver.openFileDescriptor(
uri,
"r"
)?.let { PdfRenderer(it) }
)?.let { viewModel.handleIntent(UiIntent.Init(PdfRenderer(it))) }
}

val tabBarState = rememberTopBarState()

val listState = rememberLazyListState()

renderer?.let {
PdfScreen(
renderer = it,
listState = listState,
isTopBarVisible = tabBarState.topBarVisible,
onPdfBodyPressed = tabBarState::show,
onPageIndexChange = { page ->
scope.launch {
listState.scrollToItem(page - 1)
}
},
)
}
PdfScreen(
uiState = uiState,
listState = listState,
handleIntent = viewModel::handleIntent,
onPageIndexChange = { page ->
scope.launch {
listState.scrollToItem(page - 1)
}
},
)

}

@Composable
private fun PdfScreen(
renderer: PdfRenderer,
uiState: UiState = UiState(),
listState: LazyListState = rememberLazyListState(),
pdfState: PdfToBitmap = rememberPdfState(renderer),
isTopBarVisible: Boolean = false,
onPdfBodyPressed: () -> Unit,
handleIntent: (UiIntent) -> Unit = {},
onPageIndexChange: (Int) -> Unit = {},
) {
val bitmaps = pdfState.bitmap.collectAsStateWithLifecycle()

Box(
modifier = Modifier
.fillMaxSize()
Expand All @@ -114,29 +109,29 @@ private fun PdfScreen(
.pointerInput(null) {
detectTapGestures(
onTap = {
onPdfBodyPressed()
handleIntent(UiIntent.ShowTopBar)
},
onPress = {
onPdfBodyPressed()
handleIntent(UiIntent.ShowTopBar)
}
)
},
state = listState,
verticalArrangement = Arrangement.spacedBy(4.dp)
) {
items(renderer.pageCount) { page ->
items(uiState.totalPage) { page ->
LaunchedEffect(page) {
pdfState.renderPage(page)
handleIntent(UiIntent.RenderPage(page))
}

PdfImage(
bitmap = bitmaps.value[page],
bitmap = uiState.bitmaps[page],
)
}
}

AnimatedVisibility(
visible = isTopBarVisible,
visible = uiState.topBarState.topBarVisible,
enter = slideIn {
IntOffset(0, -it.height)
},
Expand All @@ -146,8 +141,11 @@ private fun PdfScreen(
) {
PdfTopBar(
currentPage = listState.firstVisibleItemIndex + 1,
totalPage = renderer.pageCount,
onPageIndexChange = onPageIndexChange
totalPage = uiState.totalPage,
onPageIndexChange = {
handleIntent(UiIntent.ChangePage(it))
onPageIndexChange(it)
}
)
}
}
Expand Down
Loading
Loading