-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix/tech stack #86
Fix/tech stack #86
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
switch (e.keyCode) { | ||
// UP KEY | ||
case 38: | ||
nowIndex = Math.max(nowIndex - 1, 0); | ||
break; | ||
|
||
// DOWN KEY | ||
case 40: | ||
nowIndex = Math.min(nowIndex + 1, matchDataList.length - 1); | ||
break; | ||
|
||
// ENTER KEY | ||
case 13: |
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.
주석으로 keyCode가 뭔지 알려준 것 좋네요 !!
선호도 차이 같은데 저는 주석보다 코드 상단이나 하단에 상수화하는 것을 선호하는데, 참고만 해주세요 ㅎㅎ
const UP_KEY_CODE = 38
const ENTER_KEY_CODE = 13
switch(e.keyCode){
case UP_KEY_CODE:
nowIndex = Math.max(nowIndex - 1, 0);
break;
}
const newMacthList = matchDataList.slice(0, 4).map((data, index) => { | ||
if (index === nowIndex) { | ||
return { text: data, index, isOver: true }; | ||
} | ||
return { text: data, index, isOver: false }; | ||
}); |
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.
이 코드만 봤을 때 왜 앞에 4개만 잘라서 map()메서드를 사용하는지 모르겠네요 제가 생각하는 이유는 기술스택 검색 추천 리스트를 첫 4가지 요소에 대한 옵션을 제공 하기 위해서 같은데 맞을까요 ?? 코드 상단에 주석으로 이 로직이 의미하는 이유를 작성해주시면 좋을 것 같아요
// 기술스택 검색 추천 리스트를 첫 4가지 요소에 대한 옵션을 제공
const newMatchList = matchDataList.slice(0, 4).map((data, index) => ({
text: data,
index,
isOver: index === nowIndex // 이런 식으로도 작성 가능하겠네요!
}));
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.
지금은 데이터가 적어서 괜찮을 것 같은데 나중에 데이터가 많으면 한 글자 입력하면 연관 데이터가 너무 많이 나와서 카톡으로 얘기해보고 상단 4개만 거르자고 해서 slice했던 것 같네요 주석도 달아 놓을게요!!
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.
아하 그렇군요 ㅎㅎ 저도 기술스택 검색시 상단에 4개만 추천하는 것을 카톡으로 얘기한 것을 알고있었지만
이코드가 톡에서 얘기한 기술스택 4개 표시하는 부분인지 모를수도 있어서 였습니다!
바로 반경 감사합니다~
{ | ||
text: '', | ||
index: 0, | ||
isOver: false, | ||
}, |
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.
기존에 빈 배열의 데이터를 가지고 있었다가 초기값에
{
text: '',
index: 0,
isOver: false,
}
데이터를 추가하신 이유가 어떻게 될까요??
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.
객체처럼 조금 복잡한 데이터가 들어오면 어떤 형식으로 들어올거다라고 알려주는게 습관되어있던 것 같네요 근데 위에 타입으로 보여서 이것도 지우는걸로 반영하겠습니다~!
💬 PR 타입 (하나 이상의 PR 타입을 선택해주세요)
🌵 반영 브랜치
fix/tech-stack -> dev
🔎 작업 내용
✨ 영상
2024-04-10.4.36.17.mov