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

[ui] 알림 / 알림 뷰 디자인 #123

Merged
merged 24 commits into from
Aug 19, 2023
Merged

Conversation

Sangwook123
Copy link
Contributor

@Sangwook123 Sangwook123 commented Aug 18, 2023

📝 Work Description

알림 뷰 디자인
마이페이지 리디자인

📣 To Reviewers

내일까지 부탁드립니다 !

@Sangwook123 Sangwook123 added ui 🎨 UI 관련 작업 상욱 🐨 labels Aug 18, 2023
@Sangwook123 Sangwook123 added this to the 뷰 구현 🎨 milestone Aug 18, 2023
@Sangwook123 Sangwook123 self-assigned this Aug 18, 2023
Copy link
Member

@leeeha leeeha left a comment

Choose a reason for hiding this comment

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

리뷰 참고해주세요 :) 고생했어용 👍

import com.android.go.sopt.winey.util.view.ItemDiffCallback

class NotificationAdapter :
PagingDataAdapter<Notification, NotificationAdapter.NotificationViewHolder>(DiffUtil) {
Copy link
Member

Choose a reason for hiding this comment

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

알림 리스트는 페이지로 구분되어 있지 않다고 해요! 페이징 대신 그냥 일반적인 ListAdapter 사용하면 될 거 같아요!

"COMMENTNOTI" -> R.string.notification_comment
"HOWTOLEVELUP" -> R.string.notification_how_to_levelup
else -> null
}
Copy link
Member

Choose a reason for hiding this comment

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

_ (언더바) 이용해서 가독성을 좀더 높여주는 건 어떨까요??
ex) RANK_UP_TO_2 DELETE_RANK_DOWN_TO_1 GOAL_FAILED LIKE_NOTI

Copy link
Member

Choose a reason for hiding this comment

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

NotiType 이라는 enum 클래스를 만들어서, 타입을 구분하면 좋을 거 같아요! 그러면 이렇게 문자열을 하드코딩 하지 않아도 돼서 매우 편하답니다 :)

util/code 패키지에 있는 NicknameErrorCode 라는 enum class 참고해주세요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

현재는 서버에서 날아오는 NotiType의 항목들이 코드에 적힌 "RANKUPTO2", "COMMENTNOTI" 등의 문자열로 날아오기 때문에 그에 따라서 분기 처리 해주기 위해서 지금처럼 적어두었습니다 ! enum 클래스를 사용하여 구현하는 방식은 알림 서버통신을 구현하면서 제대로 처리되는지 확인해보면서 시도해보겠습니다 !

Copy link
Member

Choose a reason for hiding this comment

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

앗 그렇군요! 화긴화긴

if (drawableResourceId != 0) {
setImageResource(drawableResourceId)
} else {
}
Copy link
Member

Choose a reason for hiding this comment

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

바인딩 어댑터 활용 굿!!

android:id="@+id/tv_notification_title"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="알림"
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

@sxunea sxunea left a comment

Choose a reason for hiding this comment

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

다 확인완료 ! 그밖의 수정사항은 없어도될것같아요 고생했습니다 👍🏻

Copy link
Member

@leeeha leeeha left a comment

Choose a reason for hiding this comment

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

깃헙에서 바로 충돌 해결했더니 제대로 파일들이 합쳐지지 않아서 빌드 오류나고 있어..!!
상욱오빠가 로컬에서 충돌 다시 해결하고 푸시해주라 🙏

"COMMENTNOTI" -> R.string.notification_comment
"HOWTOLEVELUP" -> R.string.notification_how_to_levelup
else -> null
}
Copy link
Member

Choose a reason for hiding this comment

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

앗 그렇군요! 화긴화긴

@Sangwook123 Sangwook123 merged commit 7a55d33 into develop Aug 19, 2023
1 check passed
@Sangwook123 Sangwook123 deleted the feature/ui-notification-view branch September 7, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui 🎨 UI 관련 작업 상욱 🐨
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[ui] 알림 / 알림 뷰 디자인
3 participants