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

[Feature/home_network] home/store, home/mypage 네트워크 작업 #79

Merged
merged 10 commits into from
Jul 6, 2023

Conversation

librarywon
Copy link
Contributor

이슈 코드

📸 스크린샷

스크린샷 2023-07-06 오전 4 15 46 스크린샷 2023-07-06 오전 4 15 36

🍀 관련 이슈

  • store와 mypage 네트워크 작업을 완료하였습니다.

@librarywon librarywon added 🦈서재원 서재원이 했다 ✨ Feature 기능 개발 labels Jul 5, 2023
@librarywon librarywon self-assigned this Jul 5, 2023
@librarywon librarywon requested a review from a team as a code owner July 5, 2023 19:18
Copy link
Member

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

👍🏻 리뷰 확인해주세요.

@@ -64,7 +64,7 @@
android:exported="false" />
<activity
android:name=".feature.home.HomeActivity"
android:exported="false" />
android:exported="true" />
Copy link
Member

Choose a reason for hiding this comment

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

이거 다시 false로 변경

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!

@@ -54,6 +55,7 @@ object NetModule {
): OkHttpClient = OkHttpClient.Builder()
.addInterceptor(logInterceptor)
.addInterceptor(authInterceptor)
.apply { FlipperInitializer.initOkHttpClient(this) }
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

override suspend fun getMyPageInfo(): Result<MyPageResponse> {
return runCatching { retrofitMyPageNetwork.getMyPages() }
}

Copy link
Member

Choose a reason for hiding this comment

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

빈줄!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

감사합니다 ㅎㅎ

Comment on lines 73 to 75
val intent = Intent(context, AlbumListActivity::class.java).apply {
putExtra("albumId", albumItem.id)
}
Copy link
Member

Choose a reason for hiding this comment

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

이거 AlbumListActivity내의 로직으로 전환하는게 좋을듯 cc. @KwakEuiJin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그럼 일단 �인텐트로 엑티비티 전환처리만 해두겠습니다


binding.viewpagerStore.adapter = adapter
private fun setupViewPager() {
storeAdapter = StoreAdapter({ albumItem ->
Copy link
Member

Choose a reason for hiding this comment

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

trailing lambda로 처리해주실래유?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!!

storeAdapter?.submitList(storeState.data)

//최초 데이터 세팅
binding.tvStoreAlbumPhotoCount.text = storeState.data[0].photoCount.toString() + "/30"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
binding.tvStoreAlbumPhotoCount.text = storeState.data[0].photoCount.toString() + "/30"
binding.tvStoreAlbumPhotoCount.text = storeState.data.firstOrNull().photoCount.toString() + "/30"

+) 앨범 없을때의 로직도 추가해야할듯

Copy link
Member

Choose a reason for hiding this comment

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

아래 onPageChanged 콜백하고 동일 로직인 것 같은데 확인해보실?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

확인해보니 맞습니다 수정반영 했습니다!

val text = buildSpannedString {
color(colorOf(R.color.pophory_purple)) {
textAppearance(requireContext(), R.style.TextAppearance_Pophory_HeadLineBold) {
append("포포리 앨범")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
append("포포리 앨범")
append(coloredText)

Comment on lines 44 to 49
interface OnPageChangedListener {
fun onPageChanged(albumItem: AlbumItem)
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
interface OnPageChangedListener {
fun onPageChanged(albumItem: AlbumItem)
}
fun interface OnPageChangedListener {
fun onPageChanged(albumItem: AlbumItem)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영하겠습니다!

)
}
}

Copy link
Member

Choose a reason for hiding this comment

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

빈줄

@librarywon librarywon merged commit 910a4f9 into develop Jul 6, 2023
1 check passed
@librarywon librarywon deleted the feature/home_network branch July 6, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 기능 개발 🦈서재원 서재원이 했다
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature/homeNetwork]: home Network Api 연결
2 participants