-
Notifications
You must be signed in to change notification settings - Fork 2
feat: 마비노기 테마 팔레트 추가 #8
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
base: main
Are you sure you want to change the base?
Conversation
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.
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
white: '#FFFFFF', | ||
black: '#000000', | ||
|
||
// 주요 색상 | ||
blue: '#0000C0', | ||
red: '#C00000', | ||
yellow: '#FFFF00', | ||
|
||
// 그레이 톤 | ||
lightGray: '#D3D3D3', | ||
darkGray: '#282828', | ||
|
||
// 확장 색상 | ||
cobaltBlue: '#3678F1', | ||
brightRed: '#CF1414', | ||
bananaYellow: '#FFE062', | ||
ivoryIce: '#DEBA86', | ||
hydranPink: '#FFBFCA', | ||
chocoBrown: '#4E2E28', | ||
delegateGreen: '#7DDCC4', | ||
purple: '#800080', |
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.
global.css에서 이미 정의된 테마를 재정의하고 있는데 이유가 뭔가요?
이후 테마 컬러를 추가하거나 수정할때마다 두 파일 모두 수정해야 하나요?
현재는 SSOT원칙에 위배되는 것으로 보입니다
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.
Theme 색상을 사용할지가 확실하지 않아 우선은 분리해두었습니다. 팔레트 page 만들어두었는데, 색상 한번 리뷰해주시면 이번 PR에서 합치도록 하겠습니다.
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.
수정 후 resolve 부탁드려요
.text-theme-blue { color: var(--color-blue); } | ||
.text-theme-red { color: var(--color-red); } | ||
.text-theme-yellow { color: var(--color-yellow); } | ||
.text-theme-cobalt-blue { color: var(--color-cobalt-blue); } | ||
.text-theme-bright-red { color: var(--color-bright-red); } | ||
.text-theme-banana-yellow { color: var(--color-banana-yellow); } | ||
.text-theme-ivory-ice { color: var(--color-ivory-ice); } | ||
.text-theme-hydran-pink { color: var(--color-hydran-pink); } | ||
.text-theme-choco-brown { color: var(--color-choco-brown); } | ||
.text-theme-delegate-green { color: var(--color-delegate-green); } | ||
.text-theme-purple { color: var(--color-purple); } | ||
|
||
.bg-theme-blue { background-color: var(--color-blue); } | ||
.bg-theme-red { background-color: var(--color-red); } | ||
.bg-theme-yellow { background-color: var(--color-yellow); } | ||
.bg-theme-cobalt-blue { background-color: var(--color-cobalt-blue); } | ||
.bg-theme-bright-red { background-color: var(--color-bright-red); } | ||
.bg-theme-banana-yellow { background-color: var(--color-banana-yellow); } | ||
.bg-theme-ivory-ice { background-color: var(--color-ivory-ice); } | ||
.bg-theme-hydran-pink { background-color: var(--color-hydran-pink); } | ||
.bg-theme-choco-brown { background-color: var(--color-choco-brown); } | ||
.bg-theme-delegate-green { background-color: var(--color-delegate-green); } | ||
.bg-theme-purple { background-color: var(--color-purple); } | ||
|
||
.border-theme-blue { border-color: var(--color-blue); } | ||
.border-theme-red { border-color: var(--color-red); } | ||
.border-theme-yellow { border-color: var(--color-yellow); } | ||
.border-theme-cobalt-blue { border-color: var(--color-cobalt-blue); } | ||
.border-theme-bright-red { border-color: var(--color-bright-red); } | ||
.border-theme-banana-yellow { border-color: var(--color-banana-yellow); } | ||
.border-theme-ivory-ice { border-color: var(--color-ivory-ice); } | ||
.border-theme-hydran-pink { border-color: var(--color-hydran-pink); } | ||
.border-theme-choco-brown { border-color: var(--color-choco-brown); } | ||
.border-theme-delegate-green { border-color: var(--color-delegate-green); } | ||
.border-theme-purple { border-color: var(--color-purple); } |
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.
tailwind를 사용중이라 동일한 색상을 사용중일 경우 이렇게 직접 border, text별로 하나하나 설정할 필요 없이 tailwind.config에서 테마 컬러를 설정해주면 색상별 유틸리티 글래스를 자동으로 생성해줍니다
tailwind.config가 아니라 global.css에서 테마 컬러를 적용하고 싶으면 @theme를 사용할 수 있습니다
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에서 수정해도 괜찮을까요?
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에서 수정하려는 이유가 있나요?
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.
디스코드에서 이야기 진행
dev-ant: 잘 알지 못하는 부분이고 얼마나 걸릴지 몰라 머지 이후 작업하려고 했다.
mental: 얼마 안걸리고 찾아보면 바로 적응 가능하다.
dev-ant: 알겠다 현 요청에 반영 후 머지하겠다.
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.
문제없으면 머지하기 전에 resolve해주세요
기본적인 마비노기 테마 색상 팔레트 추가