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

✨ implement category layout #89

Merged
merged 4 commits into from
Jul 25, 2024
Merged

✨ implement category layout #89

merged 4 commits into from
Jul 25, 2024

Conversation

kmkim2689
Copy link
Contributor

implement category layout

  • add and update background files
    • bg_radius_filled
    • bg_radius_outlined
    • bg_radius_default
  • add category items
  • add category fragment layout

@kmkim2689 kmkim2689 added ✨ feature new feature AN Android 💄 UI labels Jul 25, 2024
@kmkim2689 kmkim2689 requested review from Hogu59 and ii2001 July 25, 2024 02:08
@kmkim2689 kmkim2689 self-assigned this Jul 25, 2024
Copy link

@Hogu59 Hogu59 left a comment

Choose a reason for hiding this comment

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

기능 구현에 고생이 많네요 케이엠.

이번에도 몇가지 리뷰 해두었으나, 필수적인 사항이라기 보다는 의견공유에 대한 사항이 많으니 보시고 괜찮다 싶으시면 코드 변경 해주시면 감사하겠습니다.

수고하셨습니다 :)


class CategoryAdapter(
private val categoryEventListener: CategoryEventListener,
) : ListAdapter<Category, CategoryViewHolder>(diffUtil) {
Copy link

Choose a reason for hiding this comment

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

ListAdapter 활용 굿입니다 bb

package net.pengcook.android.presentation.category

interface CategoryEventListener {
fun onNavigateToRecipesByCategory(categoryId: Long)
Copy link

Choose a reason for hiding this comment

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

이 함수는 카테고리가 클릭되었을때 이동하는 내용을 추상화한 함수같은데, 네이밍이 너무 명시적이어서 문제가 될 소지가 있지 않을까 싶습니다.

오히려 추상화를 한다면 행위 자체에 초점을 맞추어 "onCategorySelected"와 같은 네이밍은 어떨까 싶습니다 :)


override fun onDestroy() {
super.onDestroy()
_binding = null
Copy link

Choose a reason for hiding this comment

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

onDestory?????????? 장난한번 쳐봤습니다ㅋㅋ

val position = parent.getChildAdapterPosition(view)
val column = position % spanCount

if (includeEdge) {
Copy link

Choose a reason for hiding this comment

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

함수 분리 가능할까요? 좀 길군요..?ㅋㅋ 이중 if로 depth 도 커졌군요...

@Hogu59 Hogu59 merged commit dc15f94 into an/dev Jul 25, 2024
2 checks passed
@ii2001 ii2001 deleted the an/feat/87 branch July 26, 2024 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AN Android ✨ feature new feature 💄 UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants