-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/explore #22
Feature/explore #22
Changes from 3 commits
0d8ae9e
55f9402
c565a59
72feff4
465c1e0
c7cbefd
24a247c
8face98
c808aa9
c895fb4
ff157e5
0f5bc9b
5bb46d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
package kr.co.seedocs | ||
|
||
import android.Manifest | ||
import android.app.AppOpsManager | ||
import android.content.Intent | ||
import android.content.pm.PackageManager | ||
import android.os.Build | ||
import android.provider.Settings | ||
import androidx.activity.ComponentActivity | ||
import androidx.annotation.RequiresApi | ||
import androidx.core.app.ActivityCompat | ||
import androidx.core.content.ContextCompat | ||
|
||
private const val MANAGE_EXTERNAL_STORAGE_PERMISSION = "android:manage_external_storage" | ||
private const val MANAGE_EXTERNAL_STORAGE_PERMISSION_REQUEST = 100 | ||
private const val READ_EXTERNAL_STORAGE_PERMISSION_REQUEST = 101 | ||
|
||
internal fun checkStoragePermission( | ||
activity: ComponentActivity, | ||
): Boolean = | ||
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. Util함수로 구현하는 것 보다, 외부 스토리지 권한 로직을 캡슐화한 클래스 또는 인터페이스로 제공하는 것이 좋을 것 같습니다. 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. 추상 클래스로 변경 했습니다 |
||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { | ||
checkStoragePermissionApi30(activity) | ||
} else { | ||
checkStoragePermissionApi19(activity) | ||
} | ||
|
||
@RequiresApi(Build.VERSION_CODES.R) | ||
private fun checkStoragePermissionApi30(activity: ComponentActivity): Boolean { | ||
val appOps = activity.getSystemService(AppOpsManager::class.java) | ||
val mode = appOps.unsafeCheckOpNoThrow( | ||
MANAGE_EXTERNAL_STORAGE_PERMISSION, | ||
activity.applicationInfo.uid, | ||
activity.packageName | ||
) | ||
|
||
return mode == AppOpsManager.MODE_ALLOWED | ||
} | ||
|
||
private fun checkStoragePermissionApi19(activity: ComponentActivity): Boolean { | ||
val status = | ||
ContextCompat.checkSelfPermission(activity, Manifest.permission.READ_EXTERNAL_STORAGE) | ||
|
||
return status == PackageManager.PERMISSION_GRANTED | ||
} | ||
|
||
internal fun requestStoragePermission(activity: ComponentActivity) { | ||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { | ||
requestStoragePermissionApi30(activity) | ||
} | ||
else { | ||
requestStoragePermissionApi19(activity) | ||
} | ||
} | ||
|
||
@RequiresApi(Build.VERSION_CODES.R) | ||
private fun requestStoragePermissionApi30( | ||
activity: ComponentActivity | ||
) { | ||
val intent = Intent(Settings.ACTION_MANAGE_ALL_FILES_ACCESS_PERMISSION) | ||
|
||
activity.startActivityForResult(intent, MANAGE_EXTERNAL_STORAGE_PERMISSION_REQUEST) | ||
} | ||
|
||
fun requestStoragePermissionApi19(activity: ComponentActivity) { | ||
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. 이 함수가 public인 이유가 무엇일까요? 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. 단순 실수였습니다 바로 위 PR에서 수정했습니다 |
||
val permissions = arrayOf(Manifest.permission.READ_EXTERNAL_STORAGE) | ||
ActivityCompat.requestPermissions( | ||
activity, | ||
permissions, | ||
READ_EXTERNAL_STORAGE_PERMISSION_REQUEST | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,29 +22,39 @@ import kr.co.ui.theme.SeeDocsTheme | |
import kr.co.ui.theme.Theme | ||
import kr.co.ui.widget.FileBox | ||
import kr.co.widget.FolderBox | ||
import java.io.File | ||
|
||
@Composable | ||
internal fun ExploreRoute( | ||
path: String, | ||
padding: PaddingValues, | ||
navigateToPdf: () -> Unit = {} | ||
navigateToFolder: (String) -> Unit = {}, | ||
navigateToPdf: () -> Unit = {}, | ||
) { | ||
val files = readPDFOrDirectory(path) | ||
|
||
ExploreScreen( | ||
path = path.replace("/storage/emulated/0", "내장 저장 공간"), | ||
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. 이 컴포저블의 하드코딩된 부분은 나중에 구현되나요? |
||
files = files, | ||
padding = padding, | ||
onFolderClick = { folderPath -> navigateToFolder(folderPath) }, | ||
onFileClick = navigateToPdf | ||
) | ||
} | ||
|
||
@Composable | ||
private fun ExploreScreen( | ||
path: String, | ||
files: List<Item> = emptyList(), | ||
padding: PaddingValues, | ||
onFolderClick: (String) -> Unit = {}, | ||
onFileClick: () -> Unit = {} | ||
) { | ||
Box( | ||
modifier = Modifier | ||
.fillMaxSize() | ||
.padding(padding) | ||
.background(color = Theme.colors.bg) | ||
.background(color = Theme.colors.bg), | ||
) { | ||
LazyVerticalGrid( | ||
modifier = Modifier | ||
|
@@ -71,12 +81,13 @@ private fun ExploreScreen( | |
) | ||
Text( | ||
text = buildAnnotatedString { | ||
append("규상의 S24 >") | ||
append(">${path.split("/").dropLast(1).joinToString(separator = "/")}") | ||
append("/") | ||
withStyle( | ||
Theme.typography.caption1r.copy(color = Theme.colors.highlight) | ||
.toSpanStyle() | ||
) { | ||
append("Download") | ||
append(path.split("/").last()) | ||
} | ||
}, | ||
style = Theme.typography.caption1r, | ||
|
@@ -85,30 +96,51 @@ private fun ExploreScreen( | |
} | ||
} | ||
|
||
items(listOf("Download", "Documents", "DCIM")) { folder -> | ||
items(files.filter { it.isDirectory }) { folder -> | ||
FolderBox( | ||
name = folder | ||
name = folder.name, | ||
onClick = { onFolderClick(folder.path) } | ||
) | ||
} | ||
|
||
items( | ||
items = listOf("Effective Kotlin", "Android Developer"), | ||
items = files.filter { !it.isDirectory }, | ||
span = { GridItemSpan(maxLineSpan) } | ||
) { file -> | ||
FileBox( | ||
name = file, | ||
name = file.name, | ||
onFileClick = onFileClick | ||
) | ||
} | ||
} | ||
} | ||
} | ||
|
||
private fun readPDFOrDirectory( | ||
path: String, | ||
): List<Item> = | ||
File(path).listFiles()?.filter { !it.isHidden && (it.isDirectory || it.extension == "pdf") }?.map { | ||
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. 이 함수를 UI 쓰레드에서 호출해도 괜찮을까요? 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. 백그라운드 스레드에서 호출하는게 맞을 것 같네요 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. 네, TODO 등의 코멘트를 달고 다른 PR에서 수정하시면 될 것 같습니다. |
||
Item( | ||
name = it.name, | ||
path = it.path, | ||
type = it.extension, | ||
isDirectory = it.isDirectory | ||
) | ||
}?: emptyList() | ||
|
||
private data class Item( | ||
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. UI 모델이라면 분리된 파일 또는 패키지에 위치하는 것은 어떤가요? 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. 여러 화면에서 사용될 모델 이라 생각해서 임시로 안에 적은 뒤 |
||
val name: String, | ||
val path: String, | ||
val type: String, | ||
val isDirectory: Boolean, | ||
) | ||
|
||
@Preview | ||
@Composable | ||
private fun Preview() { | ||
SeeDocsTheme { | ||
ExploreScreen( | ||
path = "/storage/emulated/0/Download", | ||
padding = PaddingValues(), | ||
) | ||
} | ||
|
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.
onResume에서 storage permission을 체크하여 setContent를 호출하는 것은 바람직하지 않은 구조같습니다.
권한에 따라 UI의 상태가 변경되어야 한다면, 그에 따른 UI 상태를 Composable에 넘겨주어 리컴포지션할 수 있도록 구조를 변경하는게 좋을 것 같습니다.
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.
추상 클래스로 변경했습니다