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

[PC-000] Error Color 추가 및 Matching 화면 스크롤 영역 최하단 32px 여백 추가 #18

Merged
merged 3 commits into from
Jan 1, 2025

Conversation

tgyuuAn
Copy link
Member

@tgyuuAn tgyuuAn commented Dec 31, 2024

1. ⭐️ 변경된 내용

  • Error Color 추가
  • Matching 화면 스크롤 영역 최하단 32px 여백 추가

2. 🖼️ 스크린샷(선택)

image image

@tgyuuAn tgyuuAn added 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥 잡다한 🗂️ 허드렛일...! ㅌㄱ태규 ☀️ 훗날 크게될 ENFP 남성, tgyuuAn labels Dec 31, 2024
@tgyuuAn tgyuuAn requested a review from sksowk156 December 31, 2024 07:24
@tgyuuAn tgyuuAn self-assigned this Dec 31, 2024
@tgyuuAn tgyuuAn marked this pull request as ready for review December 31, 2024 07:36
Copy link
Contributor

@sksowk156 sksowk156 left a comment

Choose a reason for hiding this comment

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

태규님! 새해에도 화이팅입니다!! ㅎㅎㅎ 올 한해는 반드시 좋은 일 많으실겁니다

withStyle(style = SpanStyle(color = PieceTheme.colors.subDefault)) {
append("02:32:75")
}

append(" 남았어요")
Copy link
Contributor

Choose a reason for hiding this comment

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

  • p3 : 혹시 이것도 string.xml로 넣을 수 있는건가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • p3 : 혹시 이것도 string.xml로 넣을 수 있는건가요?
image

Spannable이 strings.xml로 분리가 가능한지 찾아보니까, xml상에서 color값까지 적용해주면 되긴 하네요,

다만 이렇게 될 경우 디자인 시스템에 정의해놓은 color를 따라가지 않아서 추후에 수정이 필요할 일이 생기면 번거로워질 것 같습니다...

Copy link
Contributor

Choose a reason for hiding this comment

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

아하 그렇군요! 그럼 이건 그냥 string으로 써두는게 낫겠죠?!

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 그렇군요! 그럼 이건 그냥 string으로 써두는게 낫겠죠?!

넵, 일단은 Spannable은 그냥 raw String으로 쓰는걸로 해요!

<string name="check_the_matching_pieces">매칭 조각을 확인해주세요!</string>
<string name="same_value_as_me">나와 같은 가치관</string>
<string name="accept_matching">매칭 수락하기</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

매칭 모듈에서 리소스를 관리하는게 나을까요??
지금 디자인 시스템에 아이콘이랑 문자열 리소스를 정의해서 사용하고 있는데요. 처음엔 아이콘을 디자인 시스템에 넣어서 중복 파일 생성을 막고 관리도 편하게 하려고 drawable 폴더에 저장했어요. 그런데 문자열은 태규님이 작성해주신 대로 매칭 모듈에서 관리하는 게 더 좋을 것 같아서 시도해봤는데, R.string...을 호출하면 기본적으로 디자인 시스템의 R이 고정되어 있어서 매칭 모듈의 R을 쓰려면 전체 파일 경로를 명시해야 하더라고요.. 허허. 그래서 일단 문자열도 디자인 시스템에 정의해서 다 불러오는 방식으로 사용 중인데, 이게 계속 고민이 되네요.

한 가지 방법으로는 import할 때 as 키워드를 사용해서 디자인 시스템의 R과 매칭 모듈의 R을 구분하는 방법이 있는데, 매번 이렇게 하는 것도 좀 귀찮고요. 혹시 더 좋은 방법이 있을까요? 아니면 그냥 매칭 모듈에서 아이콘도 다 관리하는 쪽이 나을까요? 태규님 의견이 궁금합니다!!

Copy link
Member Author

Choose a reason for hiding this comment

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

매칭 모듈에서 리소스를 관리하는게 나을까요?? 지금 디자인 시스템에 아이콘이랑 문자열 리소스를 정의해서 사용하고 있는데요. 처음엔 아이콘을 디자인 시스템에 넣어서 중복 파일 생성을 막고 관리도 편하게 하려고 drawable 폴더에 저장했어요. 그런데 문자열은 태규님이 작성해주신 대로 매칭 모듈에서 관리하는 게 더 좋을 것 같아서 시도해봤는데, R.string...을 호출하면 기본적으로 디자인 시스템의 R이 고정되어 있어서 매칭 모듈의 R을 쓰려면 전체 파일 경로를 명시해야 하더라고요.. 허허. 그래서 일단 문자열도 디자인 시스템에 정의해서 다 불러오는 방식으로 사용 중인데, 이게 계속 고민이 되네요.

한 가지 방법으로는 import할 때 as 키워드를 사용해서 디자인 시스템의 R과 매칭 모듈의 R을 구분하는 방법이 있는데, 매번 이렇게 하는 것도 좀 귀찮고요. 혹시 더 좋은 방법이 있을까요? 아니면 그냥 매칭 모듈에서 아이콘도 다 관리하는 쪽이 나을까요? 태규님 의견이 궁금합니다!!

사실 공통적으로 쓰는 리소스가 아니면 각 모듈에 배치하고 쓰는 것이 맞다고 생각하는데,

여러 개의 모듈의 R이 임포트가 되면 사용하기가 매우 번거롭다는 점은 명백히 부정할 수 없는 사실이라,

위 이슈가 해결될 때 까지 designsystem에 다 리소스를 넣어두고 개발해도 될 것 같다는 생각이 들어요.

진혁님은 어떻게 생각하시나요 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

사실 이 부분과 비슷한 고민에 대한 대화를 gdg 단톡방에서 본적이 있는데 한번 공유드려볼게요!

Image2 Image1

근데 여기서 태환님이 말씀하시는 '하나의 파일로 합쳐진다'가 정확히 무슨 의미인지는 모르겠는... 일단 한 군데서 관리하되 규칙을 명확히 정해서 쓰시는 것 같습니다! 근데 이 부분은 개발 방향에 큰 영향을 끼치는 것이 아니기에 이렇게 저렇게 해보면서 저희만의 방식을 찾아보는 것도 좋을 것 같습니다! ㅎㅎ

Copy link
Member Author

Choose a reason for hiding this comment

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

사실 이 부분과 비슷한 고민에 대한 대화를 gdg 단톡방에서 본적이 있는데 한번 공유드려볼게요!

Image2 Image1
근데 여기서 태환님이 말씀하시는 '하나의 파일로 합쳐진다'가 정확히 무슨 의미인지는 모르겠는... 일단 한 군데서 관리하되 규칙을 명확히 정해서 쓰시는 것 같습니다! 근데 이 부분은 개발 방향에 큰 영향을 끼치는 것이 아니기에 이렇게 저렇게 해보면서 저희만의 방식을 찾아보는 것도 좋을 것 같습니다! ㅎㅎ

좋아요! designsysetem에 공통으로 정의하고 각 모듈별로 주석을 달아서 구분 짓는 건 어떨까요 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

넵! 좋아요! 👍 👍

@sksowk156 sksowk156 added 머지 해도될듯염🌟 현재 코드를 기존 코드에 합쳐도 될 것 같다라고 판단..! 🌟 and removed 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥 labels Jan 1, 2025
Copy link
Member Author

Choose a reason for hiding this comment

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

@sksowk156

진혁님! 현재 파일 처럼 리소스 이름이 너무 길어지는 것 같아서 주석으로 모듈만 구분하고 앞에 prefix 단어는 빼는 걸로 하는 건 어떨까요?!

Copy link
Contributor

Choose a reason for hiding this comment

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

넵 좋아요!! 태규님이 상단에 몇 개 작성해두시면 그거에 맞춰서 작성해둘게요!

@tgyuuAn tgyuuAn force-pushed the chore/tgyuu/PC-000 branch from fc31d75 to 9e9e684 Compare January 1, 2025 12:50
@tgyuuAn tgyuuAn merged commit d563019 into develop Jan 1, 2025
1 check passed
@tgyuuAn tgyuuAn deleted the chore/tgyuu/PC-000 branch January 1, 2025 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ㅌㄱ태규 ☀️ 훗날 크게될 ENFP 남성, tgyuuAn 머지 해도될듯염🌟 현재 코드를 기존 코드에 합쳐도 될 것 같다라고 판단..! 🌟 잡다한 🗂️ 허드렛일...!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants