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: Footer Component #46
Feat: Footer Component #46
Changes from all commits
80cc04b
f908618
0fa7d46
b29a5b5
c359f4d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
그리고 지금 Footer.module.css 의 구성을 봤을 때, 모든 스타일이 footer 클래스 아래에 다 들어가 있는데요.
depth가 4개는 넘어보이고, 아래쪽 스타일을 읽다보면, 어디 단위의 p 태그를 말하는지 헷갈립니다😅
이런 방식의 스타일 작성은 일반 css 파일 하나 작성한 다음, Footer 컴포넌트 상위에 불러오는 것과 무엇이 다른 것인지 모르겠습니다. 적당한 단위로 스타일을 나눠 depth를 줄이고, CSS Modules의 특징을 활용해보는 것은 어떨까요..?
아래 예시는 참고용입니다.
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.
아마도 @Sunjae95 님께서 #50 (comment) 을 읽으시고 CSS Nesting을 활용하시려다보니 이렇게 스타일이 나오지 않으셨나 추측되네요.
저는 HTML과 CSS 간의 depth 일치해서 나란히 두고 코드를 읽으면 편하던데 개발자마다 다르게 느낄 수 있는 것 같습니다. 저는 @sounmind 님께서 제안하신 것처럼 적당한 지점에서 스타일을 나누는 방식에도 이견은 없습니다. 단, 한 가지 고려할 점은 우리가 지금 Top-down 방향으로 컴포넌트화를 시키고 있기 때문에, 나중에 이 컴포넌트의 많은 부분이 자식 컴포넌트로 스타일과 함께 빠져나갈 수 있다는 겁니다. 그러면 자연스럽게 depth는 줄 것입니다. 따라서 depth가 지나치게 많아지는 것을 컴포넌트 화가 필요한 신호로 볼 수도 있을 것 같습니다.
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.
@sounmind @DaleSeo 엇 제가 코멘트에 답변달려고 고민하던 사이에 많은일이 있었군요. PR담당자인데 사라진점 죄송합니다😅
저의 의도는 css nesting을 적극 사용하는것이고, 작성하면서 @sounmind 님과 같이 가독성이 떨어지는구나 생각도 들었습니다.
textlink같은 태그는 컴포넌트화하여 분리하면 depth가 줄여질텐데 그걸 말로 표현하기가 어려워서 늦어졌는데 @DaleSeo 님께서 잘 표현해주셨습니다.
제 생각엔 가독성이 힘들었던 부분은 media query가 들어가게되어 블록이 추가되고 child을 읽기가 힘들어졌다고 예상돼요.
또한 현재 PR에서는 앞으로 textLink가 분리되기에 depth가 더 깊어질일은 없다고 생각하구요.
위는 예시코드인데 depth가 4 depth까지가는걸 방지하고 media query를 분리하여 작성하면 가독성면에서 괜찮을까요??
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.
@sounmind @Sunjae95 대화를 통해 좋은 타협안을 찾으신 것 같군요 :) 미디어 쿼리가 안으로 들어가서 스타일이 읽기 편한 것 같습니다 💯
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.
a11y 👍