프로그래머스 고양이과제 구현 #26
프로그래머스 고양이과제 구현 #26
Conversation
| export const fetchCats = async (keyword) => { | ||
| try { | ||
| const response = await fetch(`${API_ENDPOINT}/api/cats/search?q=${keyword}`); | ||
| if (response.status === SERVER_ERROR) { |
| } | ||
| return response.json(); | ||
| } catch (error) { | ||
| alert(error); |
There was a problem hiding this comment.
이렇게하면 error.message가 '서버에러'이고, error는 Object일텐데 [object Object]라고 출력되지 않나요?
| this.$button.className = 'Button'; | ||
| this.text = text; | ||
|
|
||
| $target.appendChild(this.$button); |
There was a problem hiding this comment.
appendChild는 모던자바스크립트 튜토리얼에서 레거시라고 하더라구요. append를 사용해보심이 어떠신가요? append가 지원하는 기능이 더 많습니다!
There was a problem hiding this comment.
오호 append 찾아봐야겠네요 감사합니다!
There was a problem hiding this comment.
관련 링크 첨부합니다~ https://ko.javascript.info/modifying-document#ref-92
| this.$button.addEventListener('click', (e) => { | ||
| onClick(); | ||
| }) |
There was a problem hiding this comment.
이 코드는 이렇게 줄여볼 수도 있을 것 같네요.
this.$button.addEventListener('click', onClick);| @@ -0,0 +1,20 @@ | |||
| export default class Button{ | |||
|
|
||
| $target.appendChild(this.$loading); | ||
|
|
||
| this.isFetching && this.render(); |
There was a problem hiding this comment.
constructor는 최초 클래스 생성시에 호출되고, App.js에서 isFetching을 false로 내려주고 있어서 9번째줄의 this.render()는 호출될 일이 없어보이는데 9번째 줄의 코드가 하는 역할이 따로 있나요?
There was a problem hiding this comment.
말씀하신대로 쓸모없는 코드인데 넣어놨네요 감사합니다!
| startFetching() { | ||
| this.isFetching = true; | ||
| this.render(); | ||
| this.$loading.style.display = 'inline-block'; | ||
| } | ||
|
|
||
| finishFetching() { | ||
| this.isFetching = false; | ||
| this.$loading.style.display = 'none'; | ||
| } |
There was a problem hiding this comment.
위 리뷰의 질문과 같은 맥락인 것 같습니다. 13, 19번째줄의 this.isFetching을 조작하는 코드는 아무런 역할을 하지 않는 것 같아요.
There was a problem hiding this comment.
isFetching 굳이 필요가없는데 넣어놨군요...ㅜㅜ 감사합니다!
| constructor({ $target, onSearch, loading, setInput }) { | ||
| this.$recentKeywords = document.createElement('div'); | ||
| this.$recentKeywords.className = 'RecentKeywords'; | ||
| this.keywords = JSON.parse(localStorage.getItem('keywords')); |
There was a problem hiding this comment.
보니까 캐싱하는 작업이 keywords를 캐싱해서 다시 데이터를 불러오는 게 아니라, 그냥 배열안에 담긴 고양이 객체들 자체를 JSON.stringify로 캐싱하는 것 같아요.
지금 과정이,
새로고침 - 키워드를 바탕으로 서버로부터 데이터 받음 - 데이터에 있는 image.url을 통해 다시 서버로부터 이미지 받아옴 - 렌더링
이렇게 되는데, 그냥 이미 받았던 객체배열 자체를 캐싱하면,
새로고침 - 데이터에 있는 image.url을 통해 다시 서버로부터 이미지 받아옴 - 렌더링
이렇게 한단게 줄일 수 있습니다!
There was a problem hiding this comment.
LocalStorage엔 고양이 데이터가 들어가는게아니라 검색키워드만 들어갑니다.
따라서 절차가 새로고침 - localStorage에서 맨 앞의 키워드 뽑음 - 키워드를가지고 서버에요청 - 렌더링 순으로 진행됩니다.
localstorage에 키워드 배열들이 문자열로 저장이 되기때문에 parse해서 사용하는것입니다!
There was a problem hiding this comment.
아아 제가 말을 이상하게 했네요 ㅋㅋㅋㅋ 인서님은 그렇게 구현하신 게 맞는데,
정답은 고양이 객체들이 담긴 배열 그 자체를 캐싱하는 게 정답이지 않나 한 것이었습니다!
There was a problem hiding this comment.
"마지막 검색 결과 화면이 유지되도록 처리합니다" 이니깐
서버쪽에서 업데이트 하면 마지막 검색 결과가 달라 질 수 있으니 기존 데이터를 저장 후 쓰는게 맞는것 같아요!
| focusInput() { | ||
| setTimeout(()=> { | ||
| this.$searchInput.focus(); | ||
| }, 0) | ||
| } |
There was a problem hiding this comment.
오잉 이거 setTimeout 줘야해요? 안줘도 될 것 같은데...
아니면,
constructor에 this.$searchInput.setAttribute('autofocus', true);줘보시는 건 어떤가요?
There was a problem hiding this comment.
스택오버플로우에서 저렇게 한다고 나와있길래 한번 해봤습니다! 말씀하신대로 했는데 적용이 안되네요~ 원인을 찾아봐야겠어요
There was a problem hiding this comment.
앗... 저는 저렇게 해서 잘 되던데... 뭐가 문제일까요 궁금하네요 ㅠ
There was a problem hiding this comment.
적용이 안되는 이유가 새로고침할때마다 localStorage에서 keyword를 뽑아서 자동으로 서버에 데이터들을 요청하게됩니다. 그러면 이전에 focus했던것들이 풀리게됩니다. setTimeout으로 하게되면 eventLoop가 콜스택이 다 실행되고 마지막에 콜백을 넣어주기때문에 저렇게 setTimeout을 이용해야 Focus가 정상적으로 동작하는 것 같습니다.
| if (htmlElement.dataset.theme === 'dark') { | ||
| htmlElement.setAttribute('data-theme', 'light'); | ||
| localStorage.setItem('theme', 'light'); | ||
| return; | ||
| } | ||
| htmlElement.setAttribute('data-theme', 'dark'); | ||
| localStorage.setItem('theme', 'dark'); |
There was a problem hiding this comment.
const themeMode = htmlElement.dataset.theme === 'dark' ? 'light' : 'dark';
htmlElement.setAttribute('data-theme', themeMode);
localStorage.setItem('theme', themeMode);이렇게 리팩터링 해볼 수도 있을 것 같습니다!
There was a problem hiding this comment.
theme 자체를 os에서 받아오시면 localstorage 쓸 필요 없을것 같아요!
|
|
||
| constructor($target) { | ||
| this.$target = $target; | ||
| this.theme = localStorage.getItem('theme'); |
There was a problem hiding this comment.
이거 os 기준으로 받아온 다음에 유저가 조작 할 수 있게 만드는걸로 알고 있어요!
window.matchMedia('(prefers-color-scheme: dark)').matches
https://stackoverflow.com/questions/50840168/how-to-detect-if-the-os-is-in-dark-mode-in-browsers
| `<div class="item"> | ||
| ${index < INIT_IMAGE_CNT ? | ||
| `<img src=${cat.url} alt=${cat.name} data-index=${index} />` : | ||
| `<img class="lazy" src='' data-src=${cat.url} data-index=${index} alt=${cat.name} />`} |
There was a problem hiding this comment.
여기에 title=${cat.name} 하면
"추가 검색 결과 각 아이템에 마우스 오버시 고양이 이름을 노출합니다."
바로 해결 됩니다
HTML, CSS 관련
이미지 상세 보기 모달 관련
필수이미지를 검색한 후 결과로 주어진 이미지를 클릭하면 모달이 뜨는데, 모달 영역 밖을 누르거나 / 키보드의 ESC 키를 누르거나 / 모달 우측의 닫기(x) 버튼을 누르면 닫히도록 수정해야 합니다.추가모달 열고 닫기에 fade in/out을 적용해 주세요.검색 페이지 관련
필수데이터를 불러오는 중일 때, 현재 데이터를 불러오는 중임을 유저에게 알리는 UI를 추가해야 합니다.필수검색 결과가 없는 경우, 유저가 불편함을 느끼지 않도록 UI적인 적절한 처리가 필요합니다.필수SearchInput 옆에 버튼을 하나 배치하고, 이 버튼을 클릭할 시 /api/cats/random50 을 호출하여 화면에 뿌리는 기능을 추가합니다. 버튼의 이름은 마음대로 정합니다.추가검색 결과 각 아이템에 마우스 오버시 고양이 이름을 노출합니다.스크롤 페이징 구현
/api/cats/random50api를 요청하여 받는 결과를 별도의 섹션에 노출합니다.코드 구조 관련
필수API 의 status code 에 따라 에러 메시지를 분리하여 작성해야 합니다.