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.
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
[Feat] 디자인 시스템 초안 작성 #24
[Feat] 디자인 시스템 초안 작성 #24
Changes from 23 commits
0867c90
d62b620
6e56aad
8a54370
5ac80e4
cd77a79
69f1851
eb91bd5
349542c
c6022b7
b7595e0
5db0a66
e03dab3
8af1870
68113ed
315e093
eb988a4
30fcdf1
b0b9ff0
485d6ff
3b16da8
b3cb6aa
928bb4a
11bc8a1
b0cd9cc
b501ae9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 를 제대로 적용했으면 props 로 theme 를 가져와야 하는데 지금은 theme 를 import 하는 구조인 거 같아요!
한번 확인해주세요!!
https://emotion.sh/docs/theming
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.
아직까진 동적 테마를 사용하지 않기 때문에 상관은 없긴 할텐데,
만약 다크모드같은게 별도로 개발된다면 필요할지도 모르겠습니다.
다만 우리 사이트는 기본적으로 어두운 색 베이스고 디자이너도 고려하지 않기 때문에 필수적으로 적용할 필요는 없어보이긴 하네요 👀
stackoverflow에도 비슷한 논의가 하나 있네요.
말씀하신 방법은 props로 접근해야하기 때문에 조금 더 길어진다는 단점이 생기긴 합니다.
저는 import 방식도 충분히 괜찮다고 생각해요!
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.
저도 themeProvider가 나쁘진 않지만 import방식이 현재에는 좀 더 편리한것 같아요
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.
import 문 관련 prettier 때문에 무조건 common이 앞에 오게 되는군요... 😢
일단 현재는 이렇게 작업하고 추후 import관련 prettier를 제거하거나 해야겠네요....
@KIMSEUNGGYU 혹시 방법 아실까요?
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이 fork된 레포에는 한 번 머지된 적이 있었네요.
.prettierrc.js
의importOrder
부분을 수정하면 대응할 수 있어보입니다.다만 이렇게하면 한 줄 공백이 생기긴 하네요... 🤔
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.
맞아요ㅠㅠ import 순서 규칙을 통해 해결할 수 있지만, 그러면 공백이 생겨서.. importOrder prettier 를 제거하는것도 괜찮을수도 있겠네요 ㅎㅎ
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.
와우!! 너무 좋아졌네요!! 굿굿굿!! 👍👍