-
Notifications
You must be signed in to change notification settings - Fork 0
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
Surface 구현 #6
Surface 구현 #6
Conversation
* @param onClick Surface 클릭 시 실행되는 함수 | ||
* @param enabled Surface 클릭 가능 여부 | ||
* @param rounding Surface 모서리의 둥글기 | ||
* @param shape Surface 전체 모양. 기본값 : RectangleShape | ||
* @param backgroundColor Surface 배경 색상. 기본값 : bgBasicDefault(#0xFFFFFFFF) | ||
* @param contentColor Surface 내부 content 색상 | ||
* @param border Surface 테두리 굵기 | ||
* @param interactionSource Surface 상호작용 소스 | ||
* @param content Surface 내부 content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
각 Surface마다 달라진 param들만 적어주는 것이 가독성에 좋을 것 같긴 하네용..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다 리뷰 한 번씩 확인해주세요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surface의 use-case를 작성하는 게 아니라면 없어도 될 것 같긴 하네요
**/ | ||
|
||
@Composable | ||
@NonRestartableComposable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 @NonRestartableComposable
어노테이션을 붙인 이유가 궁금합니다! 저도 확실하진 않지만 해당 어노테이션을 여기에 붙이는 게 조금 부적절해보여서요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nonrestartablecomposable
위 링크 들어가보시면 해당 어노테이션에 대한 설명이 있는데요.. 정리하자면 성능을 위해서 불필요하게 해당 composable함수를 재실행하지 않기 위한 어노테이션 같아요.
그런데 material3에서 구현된 모든 surface에 붙어있더라구여..
사실 저도 왜.. surface에서 함수를 재실행하지 않도록 붙여야 하는거지.. 라는 의문이 있었는데,
제 조심스러운 추측으로surface composable 함수 자체가 다른 컴포넌트의 기반이 되는(레이아웃) 함수이기 때문에 변경 가능성이 적은 함수여서 그렇지 않을까 싶긴 합니다.. 저도 이걸 붙이는 게 좋을지 아닐지 많이 고민되는데 어떻게 하는 게 좋을까요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하 material3의 surface에서는 붙어있었군요. 흠 근데 일단은 관련해서 확실히 알고 있는 사람이 없다보니 일단은 제거하고, 나중에 확실히 성능 향상을 위해 붙여야겠다는 판단이 서면 그 때 붙이는 건 어떨까요?
지금은 있든 없든 크게 문제가 되진 않으니까요
저도 공부를 더 해봐야겠네욥
CompositionLocalProvider( | ||
LocalContentColor provides contentColor, | ||
) { | ||
@Suppress("DEPRECATION_ERROR") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Suppress("DEPRECATION_ERROR")
의 의미가 무엇인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 어노테이션 자체는 deprecated된 api를 썼을 때 컴파일 에러를 막게 해주는 역할로 아는데, 요기에서는 필요없는 것 같아서 지우도록 하겠습니당~~
isContainer = true | ||
} | ||
.pointerInput(Unit) {}, | ||
propagateMinConstraints = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kangyuri1114 propagateMinConstraints = true 일 필요가 있나요?
Surface를 구현했습니다.
box, button, radio button, check box, toggle 등에서 모두 필요할 것 같아 surface 먼저 구현했습니다.
material의 surface랑 YDS를 많이 참고했는데요
기존 YDS랑 달라진 점은
semantics 확장함수의 isContainer 가 deprecated 되었다고 뜨더라고요. material에서는 다음과 같이 사용 중이길래 저도 그렇게 수정했습니다.
.semantics(mergeDescendants = false) { @Suppress("DEPRECATION") isContainer = true }
기존 YDS에는 기본 surface, clickable surface 두 종류 뿐이고, checkbox나 toggle같은 곳에서는 직접 구현 중이었는데, surface 만드는 김에 material 처럼 surface, clickable surface, seleted surface, toggleable surface 4종류 구현해두었습니다. 추후 구현시에 surface를 활용하면 될 것 같아요
주석을 추가했습니다. 이런 식으로 쓰면 될지..고민이네요... 모든 params를 다 적어야 하는 게 좋을까요??
surface의 preview는 굳이.. 있을 필요가 있나 싶어 필요가 없으면 리뷰 후 지울까 고민입니다.
@NonRestartableComposable 어노테이션을 추가했습니다.
질문
질문 하나 있는데, material에서는 최소 클릭 영역을 보장하는 (각 48.dp) 확장함수를 구현해두었더라구요, 저희는 굳이 필요없을 것 같아서 안 넣었는데, 저희도 이런 확장함수가 필요할까요??
@Stable fun Modifier.minimumInteractiveComponentSize(): Modifier = this then MinimumInteractiveModifier