feat: 도서관 남은 좌석 UI 로직 #403

boiledEgg-s · 2024-12-24T15:01:18Z

https://kuring.atlassian.net/browse/KURING-228?atlOrigin=eyJpIjoiYmUzY2VmZjNmOTIxNDk2ZmJjZmU3OWVkYTFmNTkxY2UiLCJwIjoiaiJ9

요약

도서관 공지탭에 잔여좌석 화면으로 이동하기 위한 배너 추가 : 5c60581
뷰모델 구현 및 레포지토리 연결 : 95b7050
상허도서관앱 인텐트 추가 : c8b3284

잔여좌석 UI	상허도서관 이동	네트워크 오류
20241224_.mp4	20241224_.mp4	20241224_._.mp4

참고

도서관 앱 내 MainActivity가 없는 경우, 토스트 메시지를 띄우도록 자체적으로 처리했는데 이렇게 해도 될까요?
도서관 앱으로 이동하는 인텐트가 정상적으로 동작하려면 Manifest의 <queries> 에 도서관 앱 패키지명을 넣어야 하던데 이렇게 하는게 맞을까요?

mwy3055

도서관 앱이 깔려있지만 MainActivity가 없는 경우가 있을까요? 도서관 앱이 있거나 없거나 둘 중 하나일 것 같습니다.
- 도서관 앱이 있다면 지금처럼 앱을 열면 되고,
- 도서관 앱이 없다면 지금처럼 플레이 스토어의 도서관 설치 페이지를 열면 됩니다.
- 결론: 이미 잘 구현하셨습니다. 👍
Manifest에 <queries>를 반드시 넣어야 하는지? 는 모르겠습니다. 적어도 제가 알기로는 그냥 intent에 패키지명 넣어서 검색하면 되는 걸로 알고 있는데, 이 부분은 석준님이 직접 확인해 보시는 게...? (정책이 바뀌었을 수도 있으니까요)

추가로 요청사항 몇 개 드립니다.

코드 포맷이 종종 깨지는 경우가 있습니다. 커밋할 때 자동으로 reformat하도록 설정해 두시면 편합니다.
세 번째 영상에서 네트워크가 없을 때 에러 텍스트를 보여주는데, 이미 로드한 데이터가 있는 경우에는 에러 텍스트는 보여주지 말고 토스트만 보여주면 될 것 같습니다.
- 데이터 최초 로드 and 네트워크 에러 → 에러 텍스트, 토스트 둘 다 보여주기
- 데이터가 있는 상황 and 네트워크 에러 → 에러 토스트만 보여주기
(이건 그냥 의견입니다) 프리뷰는 UI 자체가 바뀔 때만 바꾸셔도 됩니다. 왜 바꾸셨나 했더니 도서관 홈페이지에서 1/5열람실만 주고 있군요 ㅋㅋ

Request changes이긴 하지만 전반적으로 매우 잘 구현되었습니다. 👍 드립니다. ㅎㅎ

mwy3055 · 2024-12-25T09:37:43Z

...rc/main/java/com/ku_stacks/ku_ring/main/notice/compose/inner_screen/NoticeHorizontalPager.kt

+            3 -> {
+                LibraryNoticeScreen(
+                    shortCategoryName = NoticeScreenTabItem.values()[pageIndex].shortCategoryName,
+                    onNavigateToLibrarySeat = onNavigateToLibrarySeat,
+                    onNoticeClick = onNoticeClick,
+                    modifier = Modifier.fillMaxSize(),
+                )
+            }


#404 에서 index 기반이 아닌 카테고리 이름 기반으로 화면을 보여주도록 수정했는데, 일단 이 PR에서는 그대로 두셔도 돼요. #404 에서 제가 수정하겠습니다.

하필 번호도 #404 군요

mwy3055 · 2024-12-25T09:38:24Z

data/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/LibraryService.kt

@@ -9,6 +9,6 @@ interface LibraryService {
    suspend fun fetchLibrarySeatStatus(
        @Query("smufMethodCode") methodCode: String,
        @Query("roomTypeId") roomTypeId: Int,
-        @Query("branchTypeId") branchTypeId: Int,
+        @Query("branchGroupId") branchGroupId: Int,


API 스키마가 바뀌었나요?

제가 제대로 확인을 못한거였습니다,, 실제로 통신해서 테스트를 한게 아니라 json파일을 직접 집어넣어서 테스트를 하다보니 쿼리스트링에서 틀린걸 인지하지 못했네요ㅎㅎ

mwy3055 · 2024-12-25T09:43:43Z

feature/library/src/main/java/com/ku_stacks/ku_ring/library/compose/LibrarySeatScreen.kt

-        androidx.compose.material.Text(
-            text = stringResource(id = com.ku_stacks.ku_ring.designsystem.R.string.notice_refresh_error_message),
+        Text(
+            text = stringResource(id = R.string.library_seats_load_fail),
            fontFamily = SfProDisplay,


SfProDisplay 글꼴을 사용하신 이유가 있나요?

수정했습니다,,

l2hyunwoo · 2024-12-25T11:43:40Z

Manifest에 를 반드시 넣어야 하는지? 는 모르겠습니다. 적어도 제가 알기로는 그냥 intent에 패키지명 넣어서 검색하면 되는 걸로 알고 있는데, 이 부분은 석준님이 직접 확인해 보시는 게...? (정책이 바뀌었을 수도 있으니까요)

안 넣으면 터집니다... (경험담)

l2hyunwoo · 2024-12-25T11:48:01Z

feature/library/src/main/java/com/ku_stacks/ku_ring/library/LibrarySeatActivity.kt

+        } catch (e: PackageManager.NameNotFoundException) {
+            throw e
+        }


에러시에 크래시틱스 로그남기는건 어떨까요?

오 넣겠습니다!

l2hyunwoo · 2024-12-25T11:50:11Z

feature/library/src/main/java/com/ku_stacks/ku_ring/library/LibrarySeatViewModel.kt

+    val uiState: StateFlow<LibrarySeatUiState>
+        get() = _uiState.asStateFlow()


Suggested change

val uiState: StateFlow<LibrarySeatUiState>

get() = _uiState.asStateFlow()

val uiState = _uiState.asStateFlow()

이건 get할 때마다 굳이 새로운 asStateFlow() 호출할 필요는 없을 것 같아요.

+) asStateFlow와 val uiState: StateFlow<아무거나> = _uiState
의 차이는 어떤 것이 있을까요?

val uiState: StateFlow<...> = _uiState의 경우 UI 단에서 MutableStateFlow로 타입캐스팅이 가능하지 않을까 추측해봅니다.

찾아보니까 asStateFlow는 ReadonlyStateFlow라는 읽기 전용 플로를 제공하여 편집이 불가능한 플로를 노출시킬 수 있다고 하네요!

이거 저한테도 알려주셨던 거 ㅋㅋ

l2hyunwoo · 2024-12-25T11:55:18Z

.../src/main/java/com/ku_stacks/ku_ring/main/notice/compose/inner_screen/LibraryNoticeScreen.kt

+    val refreshState = rememberPullRefreshState(
+        refreshing = isRefreshing,
+        onRefresh = {
+            isRefreshing = true
+            notices?.refresh()
+            isRefreshing = false
+        },
+        refreshThreshold = 75.dp,
+    )


�혹시 isRefreshing이 트리거 되면서 리컴포지션이 수행될때 이 refreshState가 다시 업데이트 되면서 isRefreshing = true 부분이 두번 호출되지 않나요? 제가 rememberPullRefreshState 내부 구현을 잘 몰라서 어떻게 될지 감이 안오네요

사실 LibraryNoticeScreen은 기존 NoticeScreen 코드를 사용하여 세부적인 동작에 관해선 제대로 인지하진 못했습니다!
지적해주신 김에 알아봐야겠네요!

l2hyunwoo · 2024-12-25T11:56:12Z

.../src/main/java/com/ku_stacks/ku_ring/main/notice/compose/inner_screen/LibraryNoticeScreen.kt

+    val noticesFlow by viewModel.noticesFlow.collectAsStateWithLifecycle()
+    val notices = noticesFlow?.collectAsLazyPagingItems()


Suggested change

val noticesFlow by viewModel.noticesFlow.collectAsStateWithLifecycle()

val notices = noticesFlow?.collectAsLazyPagingItems()

val notices = viewModel.noticesFlow.collectAsLazyPagingItems()

이렇게 한줄로 줄일 수 있지 않을까요?

viewModel.noticesFlow이 StateFlow 내부에 Flow를 포함하는 형태태여서 collect 후에 .collectAsLazyPagingItem() 적용이 가능한 것 같아요!

l2hyunwoo · 2024-12-25T11:56:42Z

.../src/main/java/com/ku_stacks/ku_ring/main/notice/compose/inner_screen/LibraryNoticeScreen.kt

+    LaunchedEffect(shortCategoryName) {
+        viewModel.getNotices(shortCategoryName)
+    }


제가 알기로는 이게 다른화면으로 갔다가 다시 이 화면으로 돌아올때에 getNotices가 호출될 수도 있거든요. 혹시 이것도 의도된 동작일까요?

매번 새로 호출해야되지 않을까요? 돌아왔을 때 새로운 공지가 생겼을 수도 있으니까요.

l2hyunwoo · 2024-12-25T11:57:43Z

...rc/main/java/com/ku_stacks/ku_ring/main/notice/compose/inner_screen/NoticeHorizontalPager.kt

+            }
+            3 -> {
+                LibraryNoticeScreen(
+                    shortCategoryName = NoticeScreenTabItem.values()[pageIndex].shortCategoryName,


Suggested change

shortCategoryName = NoticeScreenTabItem.values()[pageIndex].shortCategoryName,

shortCategoryName = NoticeScreenTabItem.�entries[pageIndex].shortCategoryName,

로 바꾸라고 워닝뜨지 않았나요?

네 근데 #404 에서 없어질 코드입니다

l2hyunwoo · 2024-12-25T11:58:26Z

feature/library/src/main/java/com/ku_stacks/ku_ring/library/compose/LibrarySeatScreen.kt

+@Composable
+private fun PagingLoadingIndicator(modifier: Modifier = Modifier) {
+    Box(modifier = modifier, contentAlignment = Alignment.TopCenter) {
+        CircularProgressIndicator(
+            color = colorResource(id = com.ku_stacks.ku_ring.designsystem.R.color.kus_green),
+            modifier = Modifier.align(Alignment.Center),
+        )
+    }
+}


이전에 만들었던거랑 중복되어서 사용하는 것 같은데 이러면 기존 베이스를 구현한 의도가 사라지지 않을까요

designsystem으로 뺐습니다!

l2hyunwoo · 2024-12-25T11:58:36Z

feature/library/src/main/java/com/ku_stacks/ku_ring/library/compose/LibrarySeatScreen.kt

-                        modifier = Modifier.wrapContentSize()
-                    )
+                is SeatLoadState.Success -> {
+                    items(seatStatus.rooms, key = { item -> item.name }) { status ->


key 설정 굳입니당

l2hyunwoo · 2024-12-25T11:59:02Z

feature/library/src/main/java/com/ku_stacks/ku_ring/library/compose/LibrarySeatScreen.kt

+    LaunchedEffect(Unit) {
+        viewModel.getLibrarySeatStatus()
+    }


이것도 아까전에 달았던 리뷰와 마찬가지

l2hyunwoo

일단 영상으로 봤을땐 괜찮은데 한번만 짚고 넘어가면 좋을 것들만 남겨놨어요

mwy3055 · 2024-12-26T01:11:39Z

Manifest에 를 반드시 넣어야 하는지? 는 모르겠습니다. 적어도 제가 알기로는 그냥 intent에 패키지명 넣어서 검색하면 되는 걸로 알고 있는데, 이 부분은 석준님이 직접 확인해 보시는 게...? (정책이 바뀌었을 수도 있으니까요)

안 넣으면 터집니다... (경험담)

업무에 큰 도움이 되었습니다. 감사합니다 (진짜임)

Android 11부터 넣도록 바뀌었네요

boiledEgg-s · 2025-01-03T17:43:39Z

2025.01.03 수정사항

PagingLoadingIndicator를 designsystem/component/indicator로 이동
건국대 도서관 앱 PackageManager.NameNotFoundException에 대한 crashlytics 메시지 추가

mwy3055

저는 괜찮아 보입니다

boiledEgg-s added 7 commits December 23, 2024 15:30

[추가] 잔여좌석화면 전환용 배너 추가

5c60581

[수정] api 쿼리파라미터 수정

d095aba

[수정] 상태창 패딩 추가

1686a5a

[제거] 주석 코드 삭제

35fd044

[추가] 뷰모델 추가 및 레포지토리 연결

95b7050

[추가] 도서관 앱 이동 로직 추가

c8b3284

[수정] 도서관 잔여좌석 오류UI 수정

69fd392

auto-assign bot requested review from l2hyunwoo and mwy3055 December 24, 2024 15:01

auto-assign bot assigned boiledEgg-s Dec 24, 2024

mwy3055 requested changes Dec 25, 2024

View reviewed changes

l2hyunwoo reviewed Dec 25, 2024

View reviewed changes

l2hyunwoo requested changes Dec 25, 2024

View reviewed changes

boiledEgg-s added 6 commits December 30, 2024 02:45

[수정] 폰트 수정

bcea715

[수정] StateFlow 선언 수정

0b29e9f

[수정] indicator 패키지 이름

da791e7

[수정] PagingLoadingIndicator 컴포넌트화

a90b662

[삭제] 미사용 라이브러리 삭제

b9b72a2

[추가] 패키지 에러 크래시리틱

ffe7a60

boiledEgg-s requested a review from mwy3055 January 3, 2025 17:42

boiledEgg-s requested a review from l2hyunwoo January 3, 2025 17:42

mwy3055 approved these changes Jan 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 도서관 남은 좌석 UI 로직 #403

feat: 도서관 남은 좌석 UI 로직 #403

boiledEgg-s commented Dec 24, 2024

mwy3055 left a comment

mwy3055 Dec 25, 2024

mwy3055 Dec 25, 2024

boiledEgg-s Dec 29, 2024

mwy3055 Dec 25, 2024

boiledEgg-s Dec 29, 2024

l2hyunwoo commented Dec 25, 2024

l2hyunwoo Dec 25, 2024

boiledEgg-s Dec 29, 2024

l2hyunwoo Dec 25, 2024

boiledEgg-s Dec 29, 2024

mwy3055 Jan 8, 2025

l2hyunwoo Dec 25, 2024

boiledEgg-s Dec 29, 2024

l2hyunwoo Dec 25, 2024

boiledEgg-s Jan 3, 2025

l2hyunwoo Dec 25, 2024

boiledEgg-s Jan 3, 2025

l2hyunwoo Dec 25, 2024

mwy3055 Dec 25, 2024

l2hyunwoo Dec 25, 2024

boiledEgg-s Jan 3, 2025

l2hyunwoo Dec 25, 2024

l2hyunwoo Dec 25, 2024

l2hyunwoo left a comment

mwy3055 commented Dec 26, 2024 •

edited

Loading

boiledEgg-s commented Jan 3, 2025

mwy3055 left a comment

		val uiState: StateFlow<LibrarySeatUiState>
		get() = _uiState.asStateFlow()

	val uiState: StateFlow<LibrarySeatUiState>
	get() = _uiState.asStateFlow()
	val uiState = _uiState.asStateFlow()

		val noticesFlow by viewModel.noticesFlow.collectAsStateWithLifecycle()
		val notices = noticesFlow?.collectAsLazyPagingItems()

	val noticesFlow by viewModel.noticesFlow.collectAsStateWithLifecycle()
	val notices = noticesFlow?.collectAsLazyPagingItems()
	val notices = viewModel.noticesFlow.collectAsLazyPagingItems()

	shortCategoryName = NoticeScreenTabItem.values()[pageIndex].shortCategoryName,
	shortCategoryName = NoticeScreenTabItem.�entries[pageIndex].shortCategoryName,

feat: 도서관 남은 좌석 UI 로직 #403

Are you sure you want to change the base?

feat: 도서관 남은 좌석 UI 로직 #403

Conversation

boiledEgg-s commented Dec 24, 2024

요약

참고

mwy3055 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l2hyunwoo commented Dec 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l2hyunwoo left a comment

Choose a reason for hiding this comment

mwy3055 commented Dec 26, 2024 • edited Loading

boiledEgg-s commented Jan 3, 2025

2025.01.03 수정사항

mwy3055 left a comment

Choose a reason for hiding this comment

mwy3055 commented Dec 26, 2024 •

edited

Loading