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

Dialog 구현 #21

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Dialog 구현 #21

wants to merge 14 commits into from

Conversation

HI-JIN2
Copy link
Member

@HI-JIN2 HI-JIN2 commented Dec 11, 2024

구현

  • Dialog를 버튼 개수에 따라 OneButtonDialog과 TwoButtonDialog로 나누어 제작하였습니다.
OneButtonDialog TwoButtonDialog
스크린샷 2024-12-11 오후 6 59 18 스크린샷 2024-12-11 오후 6 59 18

영상

default.webm

ToReviewer

Scrim

  • 명세: 해당 컬러가 다이얼로그 사용시 scrim과 함께 사용하며 scrim은 opacity/gray를 사용합니다.
  • 해당 컬러 색상이 없어서 새로 만들었는데, 피그마의 color 페이지를 봐도, Handy-Android 프로젝트 내부를 봐도 해당 컬러가 정의된 부분을 찾을 수가 없네요.. 그냥 새로 하나 추가했습니다

애니메이션

  • 피그마 문서에 해당 관련 내용이 없어서 스킵했으나, 뭔가 누르자마자 등장하니까 갑툭튀하는 느낌이 좀 들더라구요.. 어색해보이기도 하고.. 다른 분들은 괜찮게 보이나요?

@HI-JIN2 HI-JIN2 self-assigned this Dec 11, 2024
@HI-JIN2 HI-JIN2 marked this pull request as ready for review December 13, 2024 07:24
buttonType = BoxButtonType.Secondary
)

Spacer(modifier = modifier.width(dialogButtonSpacing))
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
Spacer(modifier = modifier.width(dialogButtonSpacing))
Spacer(modifier = Modifier.width(dialogButtonSpacing))

) {
// Scrim
Box(
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

특별한 이유가 있지 않다면 파라미터로 들어오는 modifier는 최상위 컴포저블에만 적용하고 나머지는 파라미터와 독립적으로 적용하는 게 관례예요

contentAlignment = Alignment.Center // 중앙 정렬
) {
Box(
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

등등

Comment on lines 68 to 71
.background(
color = HandyTheme.colors.bgBasicDefault,
shape = RoundedCornerShape(16.dp)
) //todo Radius.XL 어떻게 활용하는지 모르겠음
Copy link
Member

Choose a reason for hiding this comment

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

16.dp 대신 Radius.XL.dp 쓰시면 돼요

@HI-JIN2 HI-JIN2 requested a review from cometj03 December 16, 2024 01:57
@HI-JIN2 HI-JIN2 requested a review from a team December 31, 2024 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants