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

AttendanceCodeDialog 구현 #852

Open
wants to merge 9 commits into
base: feature/refactor-attendance-screen
Choose a base branch
from

Conversation

giovannijunseokim
Copy link
Contributor

@giovannijunseokim giovannijunseokim commented Sep 15, 2024

What is this issue?

출석 화면 리팩토링 중, AttendanceCodeDialog에 관한 중간 PR입니다.

  • AttendanceCodeDialog 구현
  • SoptTheme 임포트 할 때 솝탬프 디자인 시스템이랑,, common-ui 디자인 시스템이 자꾸 둘 중에 선택하라고 떠서 internal로 바꿔버렸씁니다

Reference

코드에 관한 질문? 지적? 환영합니다.
스크린샷 2024-09-15 오후 6 11 07

* feat: implement NewAttendanceActivity

* feat: implement NewAttendanceViewModel

* chore: add compose-lifecycle dependency

* feat: define AttendanceAction

* feat: implement screens

* feat: use SoptTheme in designsystem

* chore: data class -> class로 변경

* chore: 람다 프로퍼티 이름 명시

* chore: SoptTheme darkTheme 기본값 사용

* chore: 필요없는 함수 제거

* chore: 구현 안 된 함수에 TODO 삽입

* chore: 동작하지 않는 Preview 제거

* chore: AttendanceAction 내 뷰모델 참조 제거

* chore: code format 변경
@giovannijunseokim giovannijunseokim requested a review from a team as a code owner September 15, 2024 09:09
Copy link

height bot commented Sep 15, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@giovannijunseokim giovannijunseokim changed the title implement attendance dialog AttendanceDialog 구현 Sep 15, 2024
@giovannijunseokim giovannijunseokim changed the title AttendanceDialog 구현 AttendanceCodeDialog 구현 Sep 15, 2024
@giovannijunseokim giovannijunseokim marked this pull request as draft September 15, 2024 09:16
@giovannijunseokim giovannijunseokim changed the title AttendanceCodeDialog 구현 AttendanceCodeDialog 구현중인데 아직 문자열 추출 안 해서 나중에 봐주세요 Sep 15, 2024
@giovannijunseokim giovannijunseokim changed the title AttendanceCodeDialog 구현중인데 아직 문자열 추출 안 해서 나중에 봐주세요 AttendanceCodeDialog 구현했는데 아직 문자열 추출 안 해서 나중에 봐주세요 Sep 15, 2024
@giovannijunseokim giovannijunseokim changed the title AttendanceCodeDialog 구현했는데 아직 문자열 추출 안 해서 나중에 봐주세요 AttendanceCodeDialog 구현 Sep 15, 2024
@giovannijunseokim giovannijunseokim marked this pull request as ready for review September 15, 2024 09:21
Comment on lines +34 to +35
codes: List<String>,
inputCodes: List<String?>,
Copy link
Member

Choose a reason for hiding this comment

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

ImmutableList로 바꿔주실 수 있을까요?

AttendanceCodeCardList(
codes = inputCodes,
onTextChange = {},
onTextFieldFull = {})
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
onTextFieldFull = {})
onTextFieldFull = {}
)

}
Spacer(modifier = Modifier.height(32.dp))
Button(
onClick = { /*TODO*/ },
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 +28 to +32
if (newText.length < textMaxLength) {
onTextChange(newText)
} else {
onTextFieldFull()
}
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
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.

오케이!

Copy link
Member

@leeeyubin leeeyubin left a comment

Choose a reason for hiding this comment

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

확인했습니다~ 수고하셨어요!

Comment on lines +19 to +25
Row(modifier = modifier) {
repeat(codes.size) { index ->
AttendanceCodeCard(
text = codes[index] ?: "",
onTextChange = onTextChange,
onTextFieldFull = onTextFieldFull
)
Copy link
Member

Choose a reason for hiding this comment

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

아하 이렇게 단순 반복되는 작업은 repeat를 사용해서 나타낼 수 있군요!

Copy link
Member

Choose a reason for hiding this comment

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

수가 고정된 경우는 그냥 LazyRow 쓰는 거나 저거나 별 차이 없을거에요

} else {
onTextFieldFull()
}
}, modifier = modifier
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
}, modifier = modifier
},
modifier = modifier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants