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

[feat] subject screen ui구현 #41

Merged
merged 23 commits into from
Jan 23, 2025
Merged

[feat] subject screen ui구현 #41

merged 23 commits into from
Jan 23, 2025

Conversation

kamja0510
Copy link
Contributor

@kamja0510 kamja0510 commented Jan 18, 2025

Related issue 🛠

Work Description ✏️

  • LazyColumn 안에 grid 구현
  • 과목 추가 카드 구현

Screenshot 📸

image

Uncompleted Tasks 😅

  • 승범이가 만든 카드와 제가 만든 카드가 높이가 다름.
  • 스트링 추출
  • system background 추가

To Reviewers 📢

  • 첫번째로 승범이 카드와 제가 만든 카드의 높이가 다른 이유를 알고싶습니다.
  • 매우 미완성 상태이지만 일단 자기전에 올리고 자겠습니다. 그래도 잘 봐주시면 감사하겠습니다.

@kamja0510 kamja0510 added this to the 뷰 구현 milestone Jan 18, 2025
@kamja0510 kamja0510 self-assigned this Jan 18, 2025
@kamja0510 kamja0510 requested a review from a team as a code owner January 18, 2025 00:09
@kamja0510 kamja0510 requested review from HAJIEUN02 and beom84 January 18, 2025 21:40
Copy link
Contributor

@l2zh l2zh left a comment

Choose a reason for hiding this comment

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

잘했어요~
카드 높이는 뭔지 몰라서 못봄


init {
setEvent(SubjectContract.SubjectEvent.Initialize)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

p5: 안쓸거면 없애도 됩니다~


override fun handleEvent(event: SubjectContract.SubjectEvent) {
when (event) {
is SubjectContract.SubjectEvent.Initialize -> {}
Copy link
Contributor

Choose a reason for hiding this comment

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

마찬가지

subjectId = 5,
examRemainingDays = 1,
),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

p5: 더미 반복문으로 넣는거에 익숙해져 보면 좋을것 같아요

} else if (height == 0) {
Spacer(modifier = Modifier.width(width.dp))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

p5 : 이미 Spacer로 맞춰서 쓰고 있던걸로 아는데 지금 추가한 이유가 있을까요?

Comment on lines +57 to +65
subjectList =
state.subjectList.map { item ->
if (item.state == BbangZipCardState.CHECKABLE && item.subjectId == reduce.subjectId) {
item.copy(state = BbangZipCardState.CHECKED)
} else if (item.state == BbangZipCardState.CHECKED && item.subjectId == reduce.subjectId) {
item.copy(state = BbangZipCardState.CHECKABLE)
} else {
item
}
Copy link
Contributor

Choose a reason for hiding this comment

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

p5: 요런거 따로 분리 해주면 보기 좋겠죵?

Copy link
Contributor

Choose a reason for hiding this comment

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

p5: 최적화를 할 수 있다면 어떤 부분에서 가능할까요? (힌트 : map)
지금 하라는거 아닙니다~

@choyeongju
Copy link
Member

서버에서도 FetchType.Lazy 가 비슷하게 있는데 LazyColumn에서의 Lazy 개념도 '필요한 것만 가져와서 사용한다' -> 성능 개선 !
이런 느낌과 비슷한 걸까용?!

카드 높이가 다른 이유는.. 승범씨와 코드가 달라서 그런 거 아닐까요 ㅎㅎ

Copy link
Member

@choyeongju choyeongju left a comment

Choose a reason for hiding this comment

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

다 답해주세용!!
안드 B 시드 자리 위협중 ㅋㅋ

import org.android.bbangzip.presentation.util.modifier.applyFilterOnClick
import org.android.bbangzip.ui.theme.BbangZipTheme

@Composable
Copy link
Member

Choose a reason for hiding this comment

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

요거는 컴포넌트로 등록하는 느낌인가요?!

@Composable
fun AddSubjectCard(
modifier: Modifier = Modifier,
onClick: () -> Unit = {},
Copy link
Member

Choose a reason for hiding this comment

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

이거는 리스너같은 느낌인가요!

import org.android.bbangzip.presentation.util.base.BaseContract

class SubjectContract {
@Parcelize
Copy link
Member

Choose a reason for hiding this comment

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

반환할 때 바꿔서 주는 듯한 느낌인가요

Comment on lines +13 to +14
@Inject
constructor(
Copy link
Member

Choose a reason for hiding this comment

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

생성자 주입받는 느낌인가요?!

@@ -68,7 +68,12 @@
<!-- component - textfield -->
<string name="textfield_character_counter">%s/%s</string>

<!-- empty view -->
<string name="empty_view_text">Empty View</string>

<!-- login -->
<string name="login_title">제 과제 빵점 사장님은\n이번 학기 백점!</string>
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

@yeanssss yeanssss left a comment

Choose a reason for hiding this comment

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

코드가 직관적이라 잘 읽히는 것 같아요 우와~ 짱.

)
}

Gap(8)

Choose a reason for hiding this comment

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

코드만 봐도 뷰가 보여요! 레전드 개발자 🍞

Copy link
Contributor

@beom84 beom84 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@HAJIEUN02 HAJIEUN02 left a comment

Choose a reason for hiding this comment

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

화이팅
조금만 더 힘내보아여

@kamja0510 kamja0510 merged commit 29b1e0e into develop Jan 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feat] Subject UI 구현
6 participants