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-945] 체크박스 컴포넌트 구현 #79

Open
wants to merge 14 commits into
base: feature/redesign_bezier
Choose a base branch
from

Conversation

bonoogi
Copy link
Contributor

@bonoogi bonoogi commented Sep 25, 2024

어떤 PR인가요?

체크박스 컴포넌트를 구현하는 PR입니다.

작업 내용

  • Primary, Secondary 체크박스 구현
  • 각 체크박스에서 사용될 소스뷰 구현
  • SwiftUIExample에 예시 뷰 추가

논의 필요한 내용

  • Nested에 들어가는 내용이 한정될 수 있는지?
  • Nested의 왼쪽 패딩 영역이 어떻게 결정될 수 있는지?(피그마 상의 Nested 영역 왼쪽의 패딩과 BezierSecondaryCheckbox의 왼쪽 패딩이 서로 다름)
  • 해당 논의 진행되던 스레드 링크

스크린샷

SwiftUIExample 상에서의 예시 화면

@bonoogi bonoogi added the PR:NotYet Something isn't working label Sep 25, 2024
@bonoogi bonoogi self-assigned this Sep 25, 2024
@bonoogi bonoogi requested a review from solchan87 September 25, 2024 07:52
@bonoogi bonoogi added PR:Reviewable Improvements or additions to documentation and removed PR:NotYet Something isn't working labels Sep 25, 2024
@solchan87 solchan87 requested a review from elesahich September 30, 2024 01:38
Copy link
Contributor

@solchan87 solchan87 left a comment

Choose a reason for hiding this comment

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

피그마에서 Source 의미를 다음 싱크 때 논의를 해봐야 될 것 같아요. 피그마에서 재사용을 위해 만든 것 같은데 이번처럼 위계가 어긋나는 경우가 생기네요.
지금 구조에서 확실하게 정해졌으면 하는 점은

  • Color(blue, green) 은 피그마 상으로 어쩔 수 없이 Source의 Property 인데, 요건 명확하게 Checkbox의 Property 로 관리 되는 것이 맞을 것 같아요. ssot 를 지금은 피그마를 보고 있지만, 가급적 문서를 보는 방향으로 얘기가 되어야 할 것 같고, 문서상에 Color 가 Checkbox의 Property 로 가도록 요청 드릴까 하는데 어떠실까요?

  • Primary, Secondary 명칭을 목적에 Nested 혹은 Sub 로 변경하면 좋을 것 같아서요. 요것도 의견 주시면 같이 요청드리겠습니다.

)

self.icon?.image
.frame(length: 16)
Copy link
Contributor

Choose a reason for hiding this comment

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

Metric 추가하면 좋을 것 같아요!

import SwiftUI

struct BezierCheckboxSource: View {
private let needStroke: Bool
Copy link
Contributor

Choose a reason for hiding this comment

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

요거 사용처가 있을까요?

}
.frame(length: Metric.minHeight)

HStack(alignment: .center, spacing: 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
HStack(alignment: .center, spacing: 0) {
HStack(alignment: .center, spacing: .zero) {

채널 엑스에서는 spacing 0이 들어가는 경우가 많아서 이때마다 Metric에 추가하기 어려워서 0 일 때만 .zero로 구분해주자 했었어요. 이 컨벤션을 베지어에서도 가져갈지 논의 해보면 좋을 것 같아요

import Foundation

// MARK: - BezierCheckboxColor
public enum BezierCheckboxColor {
Copy link
Contributor

Choose a reason for hiding this comment

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

Color, Checked 는 BezierCheckbox 내부에서만 사용하고 있어서 안으로 들어가면 좋을 것 같아요!

@bonoogi
Copy link
Contributor Author

bonoogi commented Oct 18, 2024

Color(blue, green) 은 피그마 상으로 어쩔 수 없이 Source의 Property 인데, 요건 명확하게 Checkbox의 Property 로 관리 되는 것이 맞을 것 같아요. ssot 를 지금은 피그마를 보고 있지만, 가급적 문서를 보는 방향으로 얘기가 되어야 할 것 같고, 문서상에 Color 가 Checkbox의 Property 로 가도록 요청 드릴까 하는데 어떠실까요?

좋습니다!

Primary, Secondary 명칭을 목적에 Nested 혹은 Sub 로 변경하면 좋을 것 같아서요. 요것도 의견 주시면 같이 요청드리겠습니다.

용도에 따라 다른데, Secondary는 일단 알고 있기로는 독립적으로 쓸 수 없다고 해서 Sub가 낫지 않을까 생각은 하고 있습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR:Reviewable Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants