-
Notifications
You must be signed in to change notification settings - Fork 0
[refactor] 테마 컬러 리팩토링 #173
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
Conversation
HomePalette를 생성하고 이에 맞춰 HomeView Color 설정 방식을 변경했습니다.
RecordingPalette를 추가하고 RecordingView와 LoadingView를 이에 맞게 수정했습니다
ExportPalette를 생성하고 ExportView를 이에 맞게 적용했습니다
ChordProgressView를 ProgressPalette를 기반으로 색을 수정했습니다
PianoKey를 ProgressPalette에 추가하고 PianoFingeringView에 적용했습니다
Component의 색을 ComponentPalette로 빼고 BackButton, ControllerButton, ErrorView에 적용했습니다
Segyun
left a comment
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.
저는 이런 식으로 선택된 팔레트에 따라서 색상을 바꿀 수 있는 것으로 생각했었어요!
enum Palette {
case blue
case yellow
...
}
extension Palette {
var button: Color {
switch self {
case .blue: .blue1
case .yellow: .yellow
}
}
}
struct ChordProgressView: View {
let palette: Palette
var body: some View {
Button("Some Button") { ... }
.background(palette.button)
}
}
ehdrbdndns
left a comment
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.
페이지 별 팔레트로 할 시 관리 차원에서 기존과 크게 이점이 없는 것 같긴 하네요!
페이지 별로 분리된 팔레트를 palette에 통합하고, 테마 변경이 용이하도록 했습니다. 추가적으로 PaletteEnvironment와 ThemeManager를 추가해 테마를 앱 전체에서 주입받기 쉽도록 조정했습니다.
Segyun
left a comment
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.
제가 전에 제안 드린 방식처럼 팔레트 자체를 enum으로 정의하면 새로운 case를 추가하게 되면 각각의 프로퍼티의 switch 문에 새로운 case를 추가해야 되어서 번거로울 것 같긴 합니다. 그래서 다시 생각해보았을 때 팔레트를 구조체로 만들고, 정적 변수로 basic, next 등의 케이스를 추가하는 것이 어떨까 싶습니다! 새로운 팔레트 케이스를 추가할 때, 이전 팔레트는 건들지 않으면서도 확장할 수 있어서 더 좋을 것 같습니다.
struct Palette {
let backgroundColor: Color
let sheetBackground: Color
...
}
extension Palette {
static let basic: Palette = .init(
backgroundColor: .bg1,
sheetBackground: .white,
...
)
static let next: Palette = .init(
backgroundColor: .bg1,
sheetBackground: .white,
...
)
}또한 만약 뷰별로 팔레트를 구분해야 된다면 다음과 같은 식으로 분리하여 구현이 가능할 것 같습니다!
struct Palette {
let homePalette: HomePalette
let exportPalette: ExportPalette
}
extension Palette {
static let basic: Palette = .init(
homePalette: .basic,
exportPalette: .basic,
)
}
struct HomePalette {
let primaryButtonText: Color
}
extension HomePalette {
static let basic: HomePalette = .init(
primaryButtonText: .black
)
}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.
해당 작업을 새로 PR을 파서 진행해보겠습니다!
설명
전체 View의 Color를 Palette화 해서 이후 테마 변경이 용이하도록 수정했습니다.
관련 이슈
Closes #168
작업 내용
참고 내용