[westargram] main 화면 구현 3 / 코드 리뷰

Chanho Yoon·2021년 2월 26일
0

westargram

목록 보기
4/11

main 화면 구현 중 (21. 02. 26)

left-section은 어느정도 완성! 여러 사진이 넘어왔을 때 옆으로 넘어가는 건 우선 레이아웃 전체적으로 다 완성하고 추가적으로 넣기로 했다.. ㅠㅠ
이제 오른쪽 right-section 부분까지 완성해서 전체적인 레이아웃을 완성하고 게시글 사진 여러장 넘기는 것과 댓글, 좋아요 기능 구현

해야지 했는데 코드 리뷰 폭탄👍🏻


코드 리뷰... 🥲🥲🥲

으으음.. 사실 코드리뷰를 이렇게 자세히는 처음 받아봐서 당황했다.. 나름 잘 썼다고 생각했는데 고칠게 태산이다.. 사이드 부분 보다 먼저 코드 리뷰 해주신 문제점 하나하나 고치고 시작해야겠다..

코드 리뷰 고쳐할 사항들! ⚙️

✅ reset.css 하고 common.css 분리

공통으로 들어가는 속성이라고 생각하고 넣었는데 분리하는게 맞다..

✅ CSS propery convention 규칙 최대한 지키기 ⭐️⭐️

[CSS property 순서]
Layout Properties (position, float, clear, display)
Box Model Properties (width, height, margin, padding)
Visual Properties (color, background, border, box-shadow)
Typography Properties (font-size, font-family, text-align, text-transform)
Misc Properties (cursor, overflow, z-index)

login.css 규칙 적용해서 수정 완료!!
main.css 규칙 적용해서 수정 완료!!

✅ 불필요한 빈 줄은 삭제!!

변수 선언했을 때 몇 개씩 연관성 있는 변수 다음 변수는 한 칸씩 띄우는 습관이 있었는데 고치도록 해야겠다!!

✅ 가능한 불필요한 주석 삭제

정말 꼭 있어야 하는 주석 아니면 세세히 설명하는 것보다 변수명, 함수명, 클래스명을 조금 더 신경써서 주석이 없더라도 한눈에 알 수 있게 만들도록 노력해야겠다

✅ 가독성을 위해 if else문은 삼항연산자로!

  if (loginId && loginPw)
    loginBtn.style.backgroundColor = '#0095f6';
  else
    loginBtn.style.backgroundColor = '#c0dffe';
  // 아래와 같이 삼항연산자로!! 코드 4줄이 1줄로!!
  loginId && loginPw ? loginBtn.style.backgroundColor = '#0095f6' : loginBtn.style.backgroundColor = '#c0dffe';

✅ 하나로 합칠 수 있는 함수 합치기!! (중복성 제거) ⭐️⭐️

바보같이 form태그를 써놓고 idpw를 따로 받았다... 빨리 고치자

기존 코드..

getLoginId.addEventListener('keyup', ( e ) => {
  loginId = e.target.value;
  checkLoginText();
  transActionId();
});
getLoginPw.addEventListener('keyup', ( e ) => {
  loginPw = e.target.value;
  checkLoginText();
  transActionPw();
});
loginBtn.addEventListener('click', () => {
  checkLoginSubmit();
});

고친 코드..

function loginActivationCheck() {
  if(loginId && loginPw) {
    loginBtn.style.backgroundColor = '#0095f6'
    loginForm.addEventListener('submit', loginFormSubmit);
  } else {
    loginBtn.style.backgroundColor = '#c0dffe';
    loginForm.removeEventListener('submit', loginFormSubmit);
  }
}
function loignIdPwCheck( e ) {
  if (e.target.className.includes('login-id')) {
    loginId = e.target.value;
    transActionInput();
  }
  if (e.target.className.includes('login-pw')) {
    loginPw = e.target.value;
    transActionInput();
  }
  loginActivationCheck();
}
loginForm.addEventListener('keyup', loignIdPwCheck);
loginForm.addEventListener('submit', loginFormSubmit);

뭔가 더 길어진 것 같지만 하나의 함수에서 처리할 수 있는 것들은 굳이 두 개, 세 개를 나눠서 선언하고 사용하는 것보다는 더 좋다!! 이외에도 수정이 필요한 부분들은 다시 작업했다. (~ing)

✅ 최대한 의미에 맞는 시맨틱 태그 사용하기

left-sectionright-section으로 main영역을 나눴는데 left-section = main-section 이라고 하고 right-section = aside로 사용하는 게 좋았다...
css 클래스명을 바꾸는 수고를 좀 해야할 것 같다.

변경한 클래스명들

  • left-section -> main-section
  • right-section -> aside-section

변경한 태그들

  • article -> section : section안에 article이 있는 것은 어울리지 않는다는 것을 피드백 받고 수정!! (앞으로 자주 신경써야 할 부분인 것 같다)
  • div -> aside : 기존에 div태그에 right-section 클래스명이 들어가있던 걸 시맨틱 태그를 사용해서 가독성 향상

section 태그와 article 태그 다른 특성 이해하고 고치기 ⭐️⭐️

✨신영님의 피드백✨article 태그와 section 태그는 비슷해 보이지만 다른 특성을 가지고 있습니다.
section : 논리적으로 관계있는 문서 또는 요소를 분리할 때 사용합니다.
article : 문서 또는 요소가 독립적으로 존재할 수 있을 때 사용합니다.
고로 article 태그 안에 section태그는 어울리지만, section태그 안에 article태그는 어울리지 않습니다!

0개의 댓글