Skip to content

amazon search 구현#168

Merged
crongro merged 30 commits intocode-squad:baekCodefrom
77unny:search
Mar 9, 2020
Merged

amazon search 구현#168
crongro merged 30 commits intocode-squad:baekCodefrom
77unny:search

Conversation

@77unny
Copy link
Copy Markdown

@77unny 77unny commented Mar 3, 2020

데모 : https://baekcode.github.io/codesquad-FE/day4_search/search
(영어 단어 1000개 사용)

  • MVC 패턴 구현
  • JSON github 사용
  • 폼 view / 결과 view 구분
  • event delegation 사용

느낀점
MVC 패턴을 공부 하면서 완벽히 이해 되진않았지만 각각의 역할에 대해서 어느정도 알게 되었으며,
특히 컨트롤러에게 이벤트/데이터 전달하는 과정에서 많은 것을 배웠습니다. 하면서 재밌기도 했구요.

- 요청 받을때 CORS 오류, 해결
- app.hbs: 클라이언트 서버로만 구축, 테스트용
- dummy code
- 인풋 e.target.value
- 인풋 value 값, 배열 값 검색 반환
- 예) app 라고 치면 출력은 apple , appstore
- appl 라고 치면 출력은 apple 만 출력
- 네이밍 추후 변경 예정
- 검색 키워드 리스트, 해당 검색 갯수 만큼 볼드 처리
- 방향키 다운 (keyCode 40) on처리
- 방향키 업 (keyCode 38) on 처리
@crongro
Copy link
Copy Markdown
Contributor

crongro commented Mar 4, 2020

  • MVC 패턴 구현

전 이걸 미션에 요구사항으로 넣지 않았는데, 왜 다들 했는지는 모르겠지만
내가 잘 이해하는 패턴이 아니면 안쓰는게 더 좋을 수 있어요.
이왕썼으니 깊이있게 이해하고 넘어가도록 해보시고요.

@crongro
Copy link
Copy Markdown
Contributor

crongro commented Mar 4, 2020

앗 충돌이 났어요..PR이.

Copy link
Copy Markdown
Contributor

@crongro crongro left a comment

Choose a reason for hiding this comment

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

MVC를 해보신 경험은 좋긴해요. 많이 공부하고 배우셨을거 같네요.

리뷰 남긴 부분 살펴봐주세요~!

승인/머지해요

app.js Outdated
import MainController from "./js/controllers/MainController.js";

document.addEventListener('DOMContentLoaded', ()=>{
MainController.init();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

클래스는 대문자로 시작하지만, 객체는 보통 소문자로 시작.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

파일명도 대소문자로 구분한다는 말씀이신거죠? 이해했습니다!

const $ = selector => document.querySelector(selector);
const $$ = selector => document.querySelectorAll(selector);

export { $, $$ };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

유틸을 조금씩 더 늘려보세요~ 범용적인 기능을 늘려가면서요.

const JSON_DATA_URL =
'https://baekcode.github.io/codesquad-FE/day4_search/keyword.json';

export default {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

export default 하면 이름이 없어서. 이게 뭐지?? 라는 생각이 들긴하네요.

this.resultList = -1;
SearchModel.setup(JSON_DATA_URL);
SearchView.setup($('#search'))
.on('@input', e => this.onInput(e.detail.input))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

on 메서드로 동작하는 것이 멋져보입니다. 👍

const listChild = Array.from(list.children);
if (this.resultList === listChild.length - 1) return false;
listChild.forEach(v => {
if (v.classList.contains('on')) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

v 라고 쓸 수 있는데, 좀더 진짜 이름을 지어져도 되죠.

},
onKeyDown() {
const list = $('.keyword-list.open');
const listChild = Array.from(list.children);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

listChild가 아니고, 복수의 표현을 쓰셔야 할 듯. listChildrens ?? 음

});
},
getData() {
return JSON.parse(localStorage.getItem('keyword_json')).keyword;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

localstorage에 데이터가 없다면 어떻게 되는거죠?

return JSON.parse(keywordDATA).keyword;
};

const searchList = (el, arr) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이 함수는 리팩토링이 필요해 보이네요.
약간 중첩된 코드같아서요.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

search.js 파일은 dummy용.. 지우는걸 잊었습니다.

@@ -0,0 +1,21 @@
export default {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

js/views/ResultView.js
라고 이름지으면 이게 slide인지 ,search인지, 아니면 Result 라는 하위서비스가 있는 서비스인지..잘 모를 듯.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

search 라는 디렉토리 만들고 그디렉토리에 위치하는 경우는 어떠한가요?

@@ -0,0 +1,75 @@
import {$,$$} from './util.js';

const search = $('#search');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MVC 가 잇는상태에서 코드내용을 잘 뜯어보지 않으면,
search.js 는 어떤 역할일지 잘 모를거 같네요.
MVC이외에 뭔가 하나가 더 있는 느낌이랄까? MVC와 이 아이와의 관계는 무엇일지도 의문이 들고요.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

search.js 파일은 dummy용.. 지우는걸 잊었습니다.

@crongro
Copy link
Copy Markdown
Contributor

crongro commented Mar 4, 2020

충돌이 나서 일단 그대로 둡니다.

@77unny
Copy link
Copy Markdown
Author

77unny commented Mar 5, 2020

앗 충돌이 났어요..PR이.

충돌 해결했습니다!

@crongro crongro merged commit 2dccf53 into code-squad:baekCode Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants