SCRUM-255 design: add music component #25
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 기존 NoteComponent.kt에서 음악 부분만 컴포넌트 파일로 분리합니다. - 작업이 더 필요한 상태입니다.
- Music 컴포넌트가 나타내야 하는 5가지의 상태를 sealed class로 추가 - MusicComponent의 Slot API 기반 레이아웃을 제작 - 각 컴포넌트 별로 맞는 레이아웃 컨텐츠를 추가
Collaborator
hyunjung-choi
left a comment
hyunjung-choi
left a comment
There was a problem hiding this comment.
다크모드는 구현 생략하신건가요 !?
Member
Author
다크모드 프리뷰 추가해놓겠습니다. 내부에는 |
Member
Author
|
다크모드 프리뷰 추가했습니다. @hyunjung-choi 현정님 코테랑 면접 화이팅입니당 |
Collaborator
hyunjung-choi
left a comment
hyunjung-choi
left a comment
There was a problem hiding this comment.
코드가 너무 깔끔해요 . . 👍
Comment on lines
+8
to
+12
| interface MusicData { | ||
| val imageUrl: String | ||
| val songName: String | ||
| val artistName: String | ||
| } |
Collaborator
There was a problem hiding this comment.
얘는 파일을 분리하는게 나을까요..? 🤔
Member
Author
There was a problem hiding this comment.
음 이 인터페이스의 역할은 아래 MusicComponentState에 공통된 세부 데이터가 들어갈 때 제약을 하는 역할 밖에 없어서 굳이 분리하기는 그래요.
이 인터페이스는 private으로 두는 건 어떨까요?
(이렇게 하면 뷰 데이터용 sealed 인터페이스의 이름을 detekt 규칙에 따라 MusicComponentData로 바꿀 거긴 해요.)
app/src/main/java/com/lyrics/feelin/core/designsystem/icon/Icons.kt
Outdated
Show resolved
Hide resolved
Member
Author
잼민이였는지 클로드였는지는 기억이 안 나지만 공통된 구조를 깔끔하게 가져갈 수 있도록 같이 고민해봤습니당 |
Replace Painter implementation with ImageVector for remaining vector icons in Icons.kt to ensure consistency and clarity. - Convert CaretIcon, EmptyImageDarkIcon, etc. to ImageVector - Remove unused Painter imports
Update Icons.kt to return ImageVector instead of Painter for vector assets. Resolve type mismatch compilation errors in LoginScreen, MyPageScreen, and UI components by replacing painter parameter with imageVector.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
What is the current behavior?
SCRUM-255
What is the new behavior (if this is a feature change)?
Music 컴포넌트를 구현합니다.
해당 컴포넌트가 사용되는 5가지 형태를 모두 포함합니다.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No breaking changes.
기존 컴포넌트에 신규 컴포넌트를 사용하는 방식으로 사용하지만, 이번에 제작한 신규 컴포넌트의 사용법과 동일합니다.
ScreenShots (If needed)
NoteComponent에 이 컴포넌트를 사용해도 형태가 변형되지 않습니다.
Other information:
마이페이지 요소를 단순히 표시하는 부분과는 약간 떨어진 컴포넌트인데, 구현한 부분은 넣는 게 맞는 듯 해 이 PR까지는 제출합니다.