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.

Sign up for GitHub

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

Jump to bottom

Feature/#886 내가 작성한 글 구현 #887

Merged

ki960213 merged 15 commits into android-main from Feature/#886-내가_작성한_글_구현

Feb 7, 2024

Collaborator

chws0508 commented Dec 17, 2023 •

edited

Loading

#️⃣ 연관된 이슈

close : #886

📝 작업 내용

내가 작성한 글 화면을 구현하였음

스크린 샷

예상 소요 시간 및 실제 소요 시간 (일 / 시간 / 분)

예상 소요 시간 : 3시간
실제 소요 시간 : 4시간

💬 리뷰어 요구사항 (선택)

지금 행사 제목이 안내려오고 있어서 함께해요 화면의 제목이 빈 상태임

chws0508 added 7 commits

December 15, 2023 15:09


          refactor : setting에 내가 작성한 글 버튼 추가

43ec42b


          refactor : Feed가 Event를 가지도록수정

5d0879e


          refactor : recruitment가 eventName을 가지도록 수정

1ad307d


          refactor : 피드와 함께가요의 ViewPager를 담당하는MyPostActivity 구현

d29da2f


          feat : 내가 작성한 피드 글 repository 기능 구현

c768a10


          feat : 내가 작성한 피드 글 fragment 구현

3bebeab


          feat : 내가 작성한 함께가요 글 구현

chws0508 added the Android label

chws0508 requested review from tmdgh1592 and ki960213

December 17, 2023 13:57

chws0508 self-assigned this

ki960213 reviewed

View reviewed changes

Collaborator

ki960213 left a comment •

edited

Loading

RefreshableViewModel을 잘 사용해주셔서 좋았습니다.
보니까 작성한 글이 서버에서 전송해준 순서 그대로 보여주던데, 저는 작성한 댓글을 최신순으로 정렬하고 있습니다. 그리고 날짜는 "yyyy.MM.dd" 형식으로 보여주고 있습니다. 작성한 글 순서 또한 최신순으로 보여주면 어떨까요?
날짜 형식은 스캇이 사용한 형식을 보여줘도 좋다고 생각합니다. 다만 작성한 글과 작성한 댓글의 날짜 형식을 통일하면 좋을 것 같아요. 작성한 댓글의 날짜 형식을 스캇이 사용한 날짜 형식인 "yyyy년 MM월 dd일 X요일"로 바꾸는 게 나을까요?

...oid/2023-emmsale/app/src/main/java/com/emmsale/presentation/ui/myFeedList/MyFeedsFragment.kt

+              import dagger.hilt.android.AndroidEntryPoint
+              @AndroidEntryPoint
+              class MyFeedsFragment : NetworkFragment<FragmentMyFeedsBinding>(R.layout.fragment_my_feeds) {

Collaborator

ki960213 Dec 18, 2023

메서드 순서를 onViewCreated에서 사용된 순서대로 나열하는 것은 어떤가요?

setupBinding
setuprecyclerView
observeMyRecruitments

...oid/2023-emmsale/app/src/main/java/com/emmsale/presentation/ui/myFeedList/MyFeedsFragment.kt Outdated

Comment on lines 25 to 27

+                      setupBinding()
+                      setupRecyclerView()
+                      observeMyRecruitments()

Collaborator

ki960213 Dec 18, 2023

제가 제안했던 컨벤션처럼 UI로직 초기화와 viewModel 관찰 로직을 구분하기 위해 setupXxx와 observeXxx를 공백으로 구분하는 것은 어떤가요?
디스커션

Member

tmdgh1592 Dec 18, 2023

관찰은 observeXxx, 뷰관련은 setupXxx 으로 통일해보자는 이야기를 이전에 했었는데, 저도 동의합니다.
스캇은 어떻게 생각하시는지 리뷰 남겨주심 좋을 것 같아요~

...oid/2023-emmsale/app/src/main/java/com/emmsale/presentation/ui/myFeedList/MyFeedsFragment.kt Outdated

+                      observeMyRecruitments()
+                  }
+                  private fun setupBinding() {

Collaborator

ki960213 Dec 18, 2023

setupDataBinding이라는 이름보다 setupBinding이 나을까요?
디스커션

...oid/2023-emmsale/app/src/main/java/com/emmsale/presentation/ui/myFeedList/MyFeedsFragment.kt Outdated

Comment on lines 34 to 38

+                  private fun observeMyRecruitments() {
+                      viewModel.myFeeds.observe(viewLifecycleOwner) { myFeeds ->
+                          myFeedAdapter.submitList(myFeeds)
+                      }
+                  }

Collaborator

ki960213 Dec 18, 2023

myFeeds를 관찰하고 있지만 메서드명은 MyRecruitments를 관찰하는 것처럼 보여요
디스커션

...oid/2023-emmsale/app/src/main/java/com/emmsale/presentation/ui/myFeedList/MyFeedsFragment.kt

+                      binding.rvMyFeeds.apply {
+                          adapter = myFeedAdapter
+                          itemAnimator = null
+                          addItemDecoration(DividerItemDecoration(context = context))

Collaborator

ki960213 Dec 18, 2023

공통 컴포넌트를 잘 사용해주셨네요 👍

android/2023-emmsale/app/src/main/java/com/emmsale/presentation/ui/splash/SplashActivity.kt Outdated

@@ @@ -87,7 +87,7 @@ class SplashActivity : ComponentActivity() { @@
                           },
                           onFailed = {
                               showToast(R.string.splash_not_installed_playstore)
-                              navigateToPlayStore()
+                            navigateToLogin()

Collaborator

ki960213 Dec 18, 2023

제가 android-main 브랜치에 navigateToPlayStore로 바꿨는데, 다시 navigateToLogin으로 바뀌어 있네요 😢
이런 경우가 예전에도 있었던 거 같은데, 혹시 fetch를 하고 나서도 이럴까요?

android/2023-emmsale/app/src/main/res/layout/fragment_my_feeds.xml

+                      android:layout_height="match_parent">
+                      <androidx.swiperefreshlayout.widget.SwipeRefreshLayout
+                          app:onRefresh1="@{() -> vm.refresh()}"

Collaborator

ki960213 Dec 18, 2023

onRefresh1 잘 사용해주셨네요! 👍
나중에 기존 onRefresh 다 없애고 나면 onRefresh1의 이름을 onRefresh로 바꿔야 할 것 같아요.

android/2023-emmsale/app/src/main/res/layout/fragment_my_feeds.xml Outdated

Comment on lines 40 to 57

+                              <ProgressBar
+                                  android:layout_width="wrap_content"
+                                  android:layout_height="wrap_content"
+                                  app:layout_constraintBottom_toBottomOf="parent"
+                                  app:layout_constraintEnd_toEndOf="parent"
+                                  app:layout_constraintStart_toStartOf="parent"
+                                  app:layout_constraintTop_toTopOf="parent"
+                                  app:visible="@{vm.networkUiState == NetworkUiState.LOADING}" />
+                              <com.emmsale.presentation.common.views.NetworkErrorView
+                                  android:layout_width="0dp"
+                                  android:layout_height="0dp"
+                                  app:visible="@{vm.networkUiState == NetworkUiState.NETWORK_ERROR}"
+                                  app:onRefresh="@{() -> vm.refresh()}"
+                                  app:layout_constraintTop_toTopOf="parent"
+                                  app:layout_constraintStart_toStartOf="parent"
+                                  app:layout_constraintEnd_toEndOf="parent"
+                                  app:layout_constraintBottom_toBottomOf="parent" />

Collaborator

ki960213 Dec 18, 2023

이 두 개 뷰는 SwipeRefreshLayout 밖에 있어도 좋을 것 같아요!

android/2023-emmsale/app/src/main/res/layout/fragment_my_recruitments.xml Outdated

Comment on lines 40 to 57

+                              <ProgressBar
+                                  android:layout_width="wrap_content"
+                                  android:layout_height="wrap_content"
+                                  app:layout_constraintBottom_toBottomOf="parent"
+                                  app:layout_constraintEnd_toEndOf="parent"
+                                  app:layout_constraintStart_toStartOf="parent"
+                                  app:layout_constraintTop_toTopOf="parent"
+                                  app:visible="@{vm.networkUiState == NetworkUiState.LOADING}" />
+                              <com.emmsale.presentation.common.views.NetworkErrorView
+                                  android:layout_width="0dp"
+                                  android:layout_height="0dp"
+                                  app:visible="@{vm.networkUiState == NetworkUiState.NETWORK_ERROR}"
+                                  app:onRefresh="@{() -> vm.refresh()}"
+                                  app:layout_constraintTop_toTopOf="parent"
+                                  app:layout_constraintStart_toStartOf="parent"
+                                  app:layout_constraintEnd_toEndOf="parent"
+                                  app:layout_constraintBottom_toBottomOf="parent" />

Collaborator

ki960213 Dec 18, 2023

이 두 개 뷰는 SwipeRefreshLayout 밖에 있어도 좋을 것 같아요!

android/2023-emmsale/app/src/main/res/layout/item_my_feed.xml

+                          android:layout_marginTop="18dp"
+                          android:layout_marginEnd="17dp"
+                          android:ellipsize="end"
+                          android:fontFamily="@font/nanum_square_bold"

Collaborator

ki960213 Dec 18, 2023

fontFamily를 적용하지 않고 textStyle="bold" 이렇게 설정해도 됩니다.
테마에 TextView와 EditText의 글꼴과 색상을 정의해놨기 때문입니다.

지금 보니 fontFamily를 사용하여 글꼴을 설정한 TextView가 많네요ㅠㅠ
이렇게 되면 글꼴 전체를 바꿀 때 불편할 것 같아요 😢
사실 글꼴을 바꿀 가능성이 거의 없다고 생각하기 때문에 지금 당장은 안바꿔도 좋을 것 같아요. 앞으로만 사용하지 않아도 좋을 것 같아요.

Member

tmdgh1592 commented Dec 18, 2023

대부분 토마스가 리뷰를 많이 해주셨네요!
스타일 관련 리뷰가 많아서 스캇의 생각에 따라 적절히 반영해주시면 좋을 것 같습니다.
저는 따로 리뷰할 부분이 없습니다.

chws0508 added 2 commits

December 19, 2023 15:36


          refactor : ktlint

caed703


          refactor : SplasHActivity에서 실패시 PlayStore로 이동하게 변경

8ca8d21

Collaborator Author

chws0508 commented Dec 19, 2023 •

edited

Loading

RefreshableViewModel을 잘 사용해주셔서 좋았습니다. 보니까 작성한 글이 서버에서 전송해준 순서 그대로 보여주던데, 저는 작성한 댓글을 최신순으로 정렬하고 있습니다. 그리고 날짜는 "yyyy.MM.dd" 형식으로 보여주고 있습니다. 작성한 글 순서 또한 최신순으로 보여주면 어떨까요? 날짜 형식은 스캇이 사용한 형식을 보여줘도 좋다고 생각합니다. 다만 작성한 글과 작성한 댓글의 날짜 형식을 통일하면 좋을 것 같아요. 작성한 댓글의 날짜 형식을 스캇이 사용한 날짜 형식인 "yyyy년 MM월 dd일 X요일"로 바꾸는 게 나을까요?

아뇨 제가 댓글과 같은 형식으로 변경하겠습니다. 그리고 최신순으로 정렬하는 것은 백엔드에서 정렬해서 주는 것이 좋아 보입니다. 일단은 임의로 제가 최신순으로 정렬을 하겠습니다


          refactor : 리뷰 반영

cece0c4

chws0508 requested a review from ki960213

December 19, 2023 07:17

ki960213 added 5 commits

January 29, 2024 00:05


          Merge remote-tracking branch 'origin/android-main' into Feature/#886-…

183f29d

…내가_작성한_글_구현

# Conflicts:
#	android/2023-emmsale/app/src/main/res/layout/fragment_setting.xml
#	android/2023-emmsale/app/src/main/res/values/strings.xml


          feat(MyPostsActivity): 탭 좌우 스크롤 가능하게 변경


          feat(item_my_feed.xml, item_my_recruitment.xml): 내 댓글 화면처럼 항목 클릭 시 리플…

df7becb

… 효과 추가


          refactor(MyPostService): 불필요한 클래스 제거

74f7203


          feat(my_xxx.xml): 내 댓글, 내 게시글, 내 모집글 ui 통일

a01a458

- lineSpacingExtra = 3dp
- ellipsize = end
- maxLines = 5

ki960213 merged commit 3f53d2b into android-main

1 check passed

ki960213 deleted the Feature/#886-내가_작성한_글_구현 branch

February 7, 2024 01:56

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Android