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

[MOB-1756] Convention 파일 추가 #53

Merged
merged 4 commits into from
Oct 16, 2024

Conversation

sodp5
Copy link
Contributor

@sodp5 sodp5 commented Aug 7, 2024

Convention을 추가합니다.

컨벤션으로 오인한 부분, 문장이 어색한 부분, 빠뜨린 부분 있는지 등 확인해주시면 좋을것 같습니다 🙇

https://www.easy-me.com/d 이 사이트에서 확인하면 확인해보기 좋습니다.

@sodp5 sodp5 self-assigned this Aug 7, 2024
Copy link

channeltalk bot commented Aug 7, 2024

@sodp5
Copy link
Contributor Author

sodp5 commented Aug 8, 2024

이 쪽의 파일 버튼을 보는 게 훨씬 좋군요
image

폴더 [[참고]](https://github.com/channel-io/bezier-compose/pull/16#issuecomment-2222027811)

- 모든 컴포넌트는 component 폴더 하위에 properties 패키지와 컴포넌트 파일로 생성합니다.
- bezier prefix를 제외하고 생성합니다.
Copy link
Contributor

Choose a reason for hiding this comment

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

bezier prefix를 제외하고 생성합니다 -> properties 패키지가 주어인가요?

CONVENTIONS.md Outdated
Comment on lines 217 to 219
enum class BezierButtonColor(
private val backgroundColor: @Composable () -> BezierColor,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

private는 서로 다른말하고 있는거 같아요?
image

Comment on lines +182 to +183
// Don't
internal fun getBackgroundColor(color: BezierButtonColor, variant: BezierButtonVariant): BezierColor {
Copy link
Contributor

Choose a reason for hiding this comment

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

이것도 함수로 만들고 매개변수를 통해 표현한거 같네요.


**Optional Modifier [[참고]](https://github.com/channel-io/bezier-compose/pull/42#discussion_r1699368350) [[참고2]](https://github.com/androidx/androidx/blob/androidx-main/compose/docs/compose-component-api-guidelines.md#modifier-parameter)**

- 제공하는 모든 디자인 컴포넌트에는 optional Modifier를 제공합니다.
Copy link
Contributor

Choose a reason for hiding this comment

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

진짜 진짜 잘 몰라서...
optional이 nullable로 해석할 수 있으니 다른 표현이 괜찮지 않나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

찾아보니까 코틀린에서는 default arguments라고 부르네요 🤔
https://kotlinlang.org/docs/functions.html#default-arguments

근데 Compose에서는 이거를 optional modifier라고 부르고 있고, (Is the first optional parameter)
optional parameter라는 말도 쓰이는 것 같아서 지금 표현이 괜찮을것 같아요
image

@sodp5 sodp5 requested a review from vvvvvoin September 19, 2024 05:16
@vvvvvoin vvvvvoin merged commit 709aa97 into channel-io:v2 Oct 16, 2024
1 check passed
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