Skip to content

Conversation

@yoonc01
Copy link
Collaborator

@yoonc01 yoonc01 commented Dec 29, 2024

요구사항

기본

  • "판다마켓" 로고 클릭 시 루트 페이지(“/”)로 이동합니다.
  • 로그인 페이지, 회원가입 페이지 모두 로고 위 상단 여백이 동일합니다.
  • input 요소에 focus in 일 때, 테두리 색상은 #3692FF입니다.
  • input 요소에 focus out 일 때, 테두리는 없습니다.
  • SNS 아이콘들은 클릭시 각각 실제 서비스 홈페이지로 이동합니다.
  • "회원가입”버튼 클릭 시 “/signup” 페이지로 이동합니다.
  • 로그인”버튼 클릭 시 “/login” 페이지로 이동합니다.

심화

  • 비밀번호 input 요소 위에 비밀번호를 확인할 수 있는 아이콘을 추가해 주세요.

주요 변경사항

  • sprint1 코드 리뷰 반영

스크린샷

2024-12-29.6.55.58.mov

멘토에게

마지막에 ul을 통해 소설 서비스을 나열하는 부분에 대해서 조언 부탁드립니다.

  • li 대신에 a를 사용했습니다. 이 점에서 list라는 의미를 잃게 될 거 같은 아쉬움이 있습니다.
<ul class="icon-list">
    <li class="circle white-background">
        <a href="https://www.google.com">
            <img  class="google-icon" src="/src/assets/icons/google_icon.png" alt="구글 로그인">
        </a>
    </li>
</ul>

위는 원래 작성했던 코드입니다. 이때 a를 제외한 li 부분 클릭 시에는 링크 이동이 동작하지 않습니다. 이거를 수정하려고 하면 추가로 작성해야 할 것들이 많아 지저분해진다고 생각했습니다.
가장 큰 원인은 icon을 만들면서 지져분해진 거 같은데 강사님 같은 경우에는 icon을 어떻게 생성하실 건지에 대한 의견 묻고 싶습니다!

@yoonc01 yoonc01 requested a review from kiJu2 December 29, 2024 10:08
@yoonc01 yoonc01 self-assigned this Dec 29, 2024
@yoonc01 yoonc01 added the 순한맛🐑 마음이 많이 여립니다.. label Dec 29, 2024
@yoonc01 yoonc01 changed the title Sprint2/효준 [윤효준] Sprint2 Dec 29, 2024
@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 31, 2024

스프리트 미션 하시느라 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다. 😊

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 31, 2024

굿굿 ! 커밋 메시지의 목적이 명확하며 단위도 깔끔하네요 ! 또한 동영상 첨부 덕에 어떤 미션을 어떻게 수행했는지 보기 좋습니다 ㅎㅎㅎ

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 31, 2024

이때 a를 제외한 li 부분 클릭 시에는 링크 이동이 동작하지 않습니다. 이거를 수정하려고 하면 추가로 작성해야 할 것들이 많아 지저분해진다고 생각했습니다.

<ul class="icon-list">
    <li class="circle white-background">
        <a href="https://www.google.com">
            <img  class="google-icon" src="/src/assets/icons/google_icon.png" alt="구글 로그인">
        </a>
    </li>
</ul>

위는 원래 작성했던 코드입니다.


li부분 클릭시에 이동이 되지 않는다는건 이미지 바깥 영역을 클릭했을 때 링크 이동이 되지 않는다는걸까요?
그렇다면 li에 패딩이 있을 것 같은데 a에 패딩을 넣는 방법이 있을 것 같아요.
혹은 ali보다 상위에 놓는 방법도 있습니다 😊

<div class="header-content">
<a href="/">
<img src="./src/assets/icons/판다 마켓 로고.svg" alt="판다 마켓 로고" />
<img src="./src/assets/icons/panda-market-logo.svg" alt="판다 마켓 로고" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굿굿 ! 파일명에 띄어쓰기, 한글이 없어졌군요. 👍

Comment on lines +47 to +57
<p class="main-content-text-header">
Hot item
</div>
<div class="main-content-text-title">
</p>
<p class="main-content-text-title">
인기 상품을 <br />
확인해 보세요
</div>
<div class="main-content-text-detail">
</p>
<p class="main-content-text-detail">
가장 HOT한 중고거래 물품을 <br />
판다 마켓에서 확인해 보세요
</div>
</p>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(의견/제안) h 태그를 활용해볼 수도 있을 것 같아요 !

기존의미는 다음과 같습니다:
내용: 인기 상품을 확인해 보세요
내용: 가장 HOT한 중고거래 물품을 판다 마켓에서 확인해 보세요


그리고 h를 적용한다면 의미가 다음과 같아집니다:
제목: 인기 상품을 확인해 보세요
내용: 가장 HOT한 중고거래 물품을 판다 마켓에서 확인해 보세요


어떤 의미가 효준님께서 나타내시고 싶은 내용인지 한 번 고민해보세요 ! 😊

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

벡터 이미지는 svg로 저장하시는걸 추천드려요 !

이하 MDN
SVG(Scalable Vector Graphics)는 2차원 벡터 그래픽을 서술하는 XML 기반의 마크업 언어입니다. SVG는 텍스트 기반의 열린 웹 표준 중 하나로, 모든 사이즈에서 깔끔하게 렌더링 되는 이미지를 서술하며 CSS, DOM, JavaScript, SMIL (en-US) 등 다른 웹 표준과도 잘 동작하도록 설계됐습니다. SVG는 달리 말하자면 HTML과 텍스트의 관계를 그래픽에 적용한 것입니다

image

이미지 파일은 최소 단위가 픽셀로 되어있으며 확대했을 때 이미지가 깨질 수 있어요 !
피그마에서 export하실 때에 svgexport할 수 있습니다 😊

Rester vs Vector

Comment on lines +20 to +21
<label for="email" class="form-label">이메일</label>
<input id="email" class="form-input" type="email" placeholder="이메일을 입력해주세요">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굿굿 ! 라벨과 인풋이 적절히 작성되었군요 !!

type, id 그리고 placeholder 등 속성들 또한 적절합니다 !
추가적으로 namerequired도 고려해볼 수 있겠네요 !

  • name: name<form>을 사용해서 추 후 submit을 사용하여 접근할 때 사용될 수 있습니다.
  • required: required<form> 내부에서 서식을 작성하지 않았을 때 브라우저에서 유저에게 피드백을 줄 수 있습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<input>에 대한 속성과 관련하여

<input>에는 용이한 속성들이 상당히 많습니다 !

  • 숫자의 범위를 지정하는 max, min
  • 글자 수를 제한하는 maxLenght, minLenght
  • 그 외 의외의 typedate, email, file, image ...
    제공되는 유용한 속성들을 모르고 개발하게 되면 자바스크립트로 만들게 되는 경우도 있어요.

예를 들어서 max라는 속성을 모르면 input 값이 입력되었을 때 값에 대한 유효성 검사를 하고 input에 값을 넣어주는 번거로운 일도 할 수도 있어요.
(제가 처음 개발할 때 그랬습니다... 🥲)

특히 리액트로 넘어가게되면 javascript가 html 접근하는 허들이 낮아지면서 위와 같은 사례가 종종 보여요.

그래서 특히 input과 관련하여서는 꼭 한번 즈음 mdn 공식 문서를 처음부터 끝까지 러프하게라도 읽어보시는걸 추천드려요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 추천 감사합니다!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(의견) global.js라고 하기에는 특정 리소스에 영향을 주는 것 같아요 !

아마도, 로그인과 회원가입 두 스크립트에서 사용하기에 global.js가 된 것 같아요. 다만, global이라는 키워드로 하기에는 특정 사용자 플로우에 더욱 가까운 것 같습니다 !
auth라는 키워드가 더 적합하지 않을까 싶어요 !

Comment on lines +34 to +35
const allFilled = Array.from(inputs).every(input => input.value.trim() !== "");
button.disabled = !allFilled;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굿굿 ! 조건부를 그대로 사용하지 않고 이름을 붙였군요 !

조건에 이름을 붙임으로서 가독성이 좋습니다 ! 👍👍

Comment on lines +11 to +22
if (input.type === "password")
{
input.type = "text";
icon.src = "/src/assets/icons/visibility_off_btn.svg";
icon.alt = "비밀번호 숨기기";
}
else
{
input.type = "password";
icon.src = "/src/assets/icons/visibility_on_btn.svg";
icon.alt = "비밀번호 보기";
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guard Clause 패턴을 사용해볼까요?:

Suggested change
if (input.type === "password")
{
input.type = "text";
icon.src = "/src/assets/icons/visibility_off_btn.svg";
icon.alt = "비밀번호 숨기기";
}
else
{
input.type = "password";
icon.src = "/src/assets/icons/visibility_on_btn.svg";
icon.alt = "비밀번호 보기";
}
if (input.type === "password") {
input.type = "text";
icon.src = "/src/assets/icons/visibility_off_btn.svg";
icon.alt = "비밀번호 숨기기";
return;
}
input.type = "password";
icon.src = "/src/assets/icons/visibility_on_btn.svg";
icon.alt = "비밀번호 보기";

Guard Clause을 사용하게 되면 조건부를 조기에 처리할 수 있게 되어 핵심 로직을 구분할 수 있다는 장점이 있습니다. 따라서 가독성을 향상시킬 수 있어요.

또한, 미리 조건에 따라서 데이터의 범위를 좁힐 수 있기에 핵심 로직에서는 보장받은 데이터를 편리하게 사용할 수 있다는 장점도 있어요 😊

Guard Clause ?

다음과 같이 조기에 함수를 끝내는 기법을 의미합니다:

function calculateDiscount(price, discountRate) {
  // Guard Clause: 가격이나 할인율이 유효하지 않으면 함수 종료
  if (price <= 0 || discountRate <= 0 || discountRate > 1) {
    console.error("Invalid price or discount rate");
    return;
  }

  // 할인된 가격 계산
  const discountedPrice = price * (1 - discountRate);
  return discountedPrice;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 Guard Clause 패턴 사용을 권하신 이유에 대해 이해가 덜 된 거 같아 코멘트 남깁니다!
if else 문은 toggle을 시키는 부분으로 둘 다 핵심 로직이고 데이터의 범위와 상관없다 생각하여 제안해주신 코드가 잘 이해가 되지 않습니다!
조금 더 자세한 설명 부탁드려도 될까요???

Copy link
Collaborator

@kiJu2 kiJu2 Dec 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금처럼 if ... else로 하셔도 됩니다 ~!
지금처럼 하시든 Guard Clause를 사용하시든 논리적으로 같은 결과가 나오게 됩니다.

if (input.type === "password") { ... }
else if (input.type === "email") { ... }
else if (input.type === "tel") { ... }
else if (input.type === "number") { 
  if (number < 18) { ... }
  if (number > 24) { ... }
}
else { ... }

위와 같이 복잡한 if로 추가 확장될 수 있을 경우 코드를 간결히 하기 위해서 Guard Clause를 사용하곤 합니다.
근데 효준님의 함수의 경우 "인풋의 패스워드와 텍스트 타입을 변경하기 위한 함수"이므로 제안 정도로 보시고 판단 하에 적용하시면 됩니다 =)

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 31, 2024

수고하셨습니다 효준님 !
코드를 최대한 가독성 좋고 깔끔하게 작성하려고자 하시는게 느껴집니다 ㅎㅎㅎ
읽는 데에 불편함이 없었어요 ! 👍👍

미션 수행하시느라 정말 수고 많으셨습니다 ! 😊

@kiJu2 kiJu2 merged commit 3c8de81 into codeit-bootcamp-frontend:Basic-윤효준 Dec 31, 2024
@yoonc01 yoonc01 deleted the sprint2/효준 branch December 31, 2024 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

순한맛🐑 마음이 많이 여립니다..

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants