feat : implemented styling for each state and refactor the code for b… #137
Conversation
Screen.Recording.2024-07-13.at.12.37.02.PM.mov피그마 나온대로 상태별 구현해봤습니다! 의도하신 바와 다른 부분이 있다면 피드백 부탁드리겠습니다~! button-link 컴포넌트에 관한 상태별 스타일링은 header에서는 관여하지 않았습니다! |
components/header.js
Outdated
| @@ -171,41 +209,30 @@ class Header extends HTMLElement { | |||
| <header> | |||
| <div class="header-content"> | |||
| <a href="/"> | |||
| <img src="images/logo.png" alt="logo" /> | |||
| <svg id="logo" width="45" height="22" viewBox="0 0 45 22" fill="none" xmlns="http://www.w3.org/2000/svg"> | |||
There was a problem hiding this comment.
어떤 연유로 외부 이미지 파일을 쓰시다가 svg 파일을 embed하시기로 하셨는지 궁금하네요.
There was a problem hiding this comment.
active일 때 img들 색상을 쉽게 변경해주기 위해서 svg파일로 변경했습니다! img를 다른 이미지로 대체해서 변경해 주는 방법도 있겠지만, 타 button에 있는 이미지들까지 마우스 클릭하는 순간만 잠시 이미지를 변경하는 방식으로하면 코드양이 너무 많아지고 오히려 복잡해지는 것 같아서 svg파일로 사용하고 바로 active 일 때 path에 접근해서 색상을 변경해주는 방식으로 통일했습니다!
There was a problem hiding this comment.
구현을 바꾸지는 않았는데, 포멧팅하거나 커밋 정리하는 과정에서 수정돼서 outdated된 것 같습니다...!
SamTheKorean
force-pushed
the
sam/135
branch
3 times, most recently
from
f4e70b9 to
6379c88
Compare
Prettier가 의도한대로 작동을 안하는 것 같은데 제가 시간내서 점검을 해보고 안되면 피드백 부탁드리겠습니다! |
SamTheKorean
force-pushed
the
sam/135
branch
4 times, most recently
from
ed5892a to
0198da2
Compare
components/header.js
Outdated
| &:focus { | ||
| outline: 3px solid #846de9; | ||
| outline-offset: 2px; | ||
| border-radius: 10px; | ||
| } | ||
|
|
||
| &:focus-visible { | ||
| outline: 3px solid #24eaca; | ||
| outline-offset: 2px; | ||
| border-radius: 10px; | ||
| } |
There was a problem hiding this comment.
outline에 들어가는 컬러코드 저희 공통 변수로 사용해주시면 더 좋을 것 같아요!
var(--primary)
var(--secondary)
There was a problem hiding this comment.
고쳤습니다! 피드백 감사합니다!
components/header.js
Outdated
| } | ||
|
|
||
| &:active svg path { | ||
| fill: #160b46; |
There was a problem hiding this comment.
여기도 마찬가지로요~ var(--text-900)
components/header.js
Outdated
| &:focus { | ||
| outline: 3px solid #846de9; | ||
| outline-offset: 2px; | ||
| border-radius: 10px; | ||
| } | ||
|
|
||
| &:focus-visible { | ||
| outline: 3px solid #24eaca; | ||
| outline-offset: 2px; | ||
| border-radius: 10px; | ||
| } | ||
|
|
||
| &:hover { | ||
| box-shadow: 2px 2px 4px rgba(0, 0, 0, 0.25); | ||
| } | ||
|
|
||
| &:active svg path { | ||
| fill: #846de9; | ||
| } |
There was a problem hiding this comment.
여기도 전체적으로 공통변수 적용되어있지 않은 부분 공통변수 사용하는걸로 수정해주시면 더 좋을 것 같습니다~
…etter readibility
|
남겨주신 피드백 전부 반영했습니다! |
…etter readibility
Checklist before merging