Refactor/1270 헤어 콘텐츠 깨짐 현상 수정 및 모바일 버전 로고 변경 #1283
Conversation
GC-Park
changed the title
| @@ -87,13 +101,14 @@ const NavBar = () => { | |||
| > | |||
| <Wrapper> | |||
| <Logo onClick={goMain} role="link" aria-label="프롤로그 홈으로 이동하기"> | |||
| <img src={LogoImage} alt="" /> | |||
| {isMobile ? <img src={MobileLogo} alt="" /> : <img src={LogoImage} alt="" />} | |||
There was a problem hiding this comment.
src 내에서 삼항 연산자를 사용하면 어떨까요?
There was a problem hiding this comment.
네 삼항 연사자를 src 내부로 수정했습니다!!!
| const handleResize = () => { | ||
| setIsMobile(mobile.matches); | ||
| }; | ||
| mobile.addEventListener('change', handleResize); | ||
| return () => { | ||
| mobile.removeEventListener('change', handleResize); | ||
| }; |
There was a problem hiding this comment.
| const handleResize = () => { | |
| setIsMobile(mobile.matches); | |
| }; | |
| mobile.addEventListener('change', handleResize); | |
| return () => { | |
| mobile.removeEventListener('change', handleResize); | |
| }; | |
| const handleResize = () => { | |
| setIsMobile(mobile.matches); | |
| }; | |
| mobile.addEventListener('change', handleResize); | |
| return () => { | |
| mobile.removeEventListener('change', handleResize); | |
| }; |
들여쓰기가 네 칸인데 prettier 설정이 잘 되어있으실까요?
There was a problem hiding this comment.
아이고.. 수정하였습니다! 또한 prettier 이용해 탭 사이즈를 2로 바꿀 수 있었습니다! 감사합니다!
| @@ -51,6 +52,9 @@ const NavBar = () => { | |||
| const [isDropdownToggled, setDropdownToggled] = useState(false); | |||
| const [isWritingDropdownToggled, setWritingDropdownToggled] = useState(false); | |||
|
|
|||
| const mobile = window.matchMedia('(max-width: 420px)'); | |||
There was a problem hiding this comment.
사소한 부분이지만 좀 더 명확한 네이밍으로 변경해보면 어떨까요? mobile만 두고 봤을 땐 mobile device라는 뜻인지 isMobile인지 헷갈릴 수 있을것 같아요.
There was a problem hiding this comment.
420px이라는 값이 상수로 빼져있으면 좋을 것 같아요~ 자칫하면 다른 곳에서는 breakpoint로 다른 값을 쓰게 될 수 있을것 같아서요!
There was a problem hiding this comment.
새로 네이밍을 완료 후 상수도 분리하였습니다!!
| @@ -51,6 +52,9 @@ const NavBar = () => { | |||
| const [isDropdownToggled, setDropdownToggled] = useState(false); | |||
| const [isWritingDropdownToggled, setWritingDropdownToggled] = useState(false); | |||
|
|
|||
| const mobile = window.matchMedia('(max-width: 420px)'); | |||
| const [isMobile, setIsMobile] = useState(false); | |||
There was a problem hiding this comment.
useState끼리 모아두면 요 컴포넌트가 어떤 state를 포함하는지 좀 더 쉽게 알 수 있을 것 같네요 ㅎㅎ 참고만 해주세요~
There was a problem hiding this comment.
저희도 같은 생각입니다!!! 😊 수정하도록 하겠습니다 감사합니다!
| @@ -40,6 +41,8 @@ const navigationConfig = [ | |||
| }, | |||
| ]; | |||
|
|
|||
| const MOBILE_SCREEN_SIZE = "420px"; | |||
There was a problem hiding this comment.
MOBILE_MAX_SIZE 어떠신가요~? 가볍게 제안드려봅니다 ㅎㅎ 그리고 해당 상수가 prolog 서비스에서 공용으로 사용하는 mobile breakpoint라면 좀더 공통적인 파일 위치에 선언되어 있음 좋지 않을까요?
There was a problem hiding this comment.
너무 좋은 것 같습니다! 모두 수정하도록 하겠습니다. 감사합니다!
|
SonarCloud Quality Gate failed. |
#️⃣연관된 이슈
📝작업 내용
💬리뷰 요구사항(선택)