[2단계 - 점심 뭐 먹지] 앵버(최승수) 미션 제출합니다.#262
Conversation
UX 피드백
|
|
igy95
left a comment
igy95
left a comment
There was a problem hiding this comment.
안녕하세요 앵버~ 리뷰 코멘트 및 논의 내용에 대한 답변 달아두었으니 확인 후 재요청 부탁드립니다 👍
src/utils/store.js
Outdated
| export function generateUniqueId() { | ||
| return crypto.randomUUID(); | ||
| } |
There was a problem hiding this comment.
유니크한 ID를 반환하는 함수가 store라는 파일명에 들어있는 게 적합하지 않은 것 같아서 다른 파일로 분리하였습니다~
src/utils/store.js
Outdated
| export function getRestaurantStorage() { | ||
| if (!localStorage.getItem("restaurant")) { | ||
| localStorage.setItem("restaurant", JSON.stringify(LIST_ITEM_CONTENTS)); | ||
| } | ||
| return JSON.parse(localStorage.getItem("restaurant")); | ||
| } | ||
|
|
||
| export function setRestaurantStorage(restaurantInformation) { | ||
| localStorage.setItem("restaurant", JSON.stringify(restaurantInformation)); | ||
| } |
There was a problem hiding this comment.
도메인과 로컬 스토리지를 별도의 모듈에 두고 도메인 계층은 이 저장소 모듈을 호출하도록 수정하였습니다!
| all: "전체", | ||
| ko: "한식", | ||
| ch: "중식", | ||
| ja: "일식", | ||
| we: "양식", | ||
| as: "아시안", | ||
| etc: "기타", |
There was a problem hiding this comment.
이런 네이밍은 되도록 지양하시면 좋겠습니다. 맥락을 알고봐도 각각의 줄임말이 무엇을 지칭하는지 한번에 파악하기 힘들기 때문이에요.
| const categoryNames = { | ||
| none: "선택해주세요.", | ||
| 5: "5분 내", | ||
| 10: "10분 내", | ||
| 15: "15분 내", | ||
| 20: "20분 내", | ||
| 30: "30분 내", | ||
| }; |
There was a problem hiding this comment.
distance라는 데이터와 *분 내라는 포맷된 문자열을 분리해보면 어떨까요?
There was a problem hiding this comment.
distance 데이터에 "분 내" 라는 문자열 값이 포함되지 않도록 분리해보았습니다!
export function formatDistance(distanceMap) {
const categoryNames = {
none: "선택해주세요.",
5: "5",
10: "10",
15: "15",
20: "20",
30: "30",
};
return distanceMap.map((distance) => `${categoryNames[distance]}분 내`);
}
src/domain/Restaurant.ts
Outdated
| updateInformation(): void { | ||
| this.#information.favorites = !this.#information.favorites; | ||
| } |
There was a problem hiding this comment.
메서드의 이름이 동작에 비해 넓은 것으로 보입니다. updateInformation이라고 하면 information의 모든 필드를 업데이트 가능한 것으로 이해될 것 같아요!
There was a problem hiding this comment.
updateInformation -> updateFavorite 으로 수정하였습니다. 감사합니다!
src/components/List.js
Outdated
| const restaurantList = new RestaurantList(storedRestaurants); | ||
| restaurantList.deleteRestaurant(restaurantElement.dataset.id); | ||
| }, | ||
| cancle: () => { |
There was a problem hiding this comment.
감사합니다, cancle-> cancel로 오타 수정하였습니다.😂
src/components/ListItem.js
Outdated
| </p> | ||
| </div> | ||
| </div> | ||
| <img src="${renderFavoritesImg(favorites)}" alt=favorites class="favorites-icon" /> |
There was a problem hiding this comment.
alt에""이 필요해 보이네요- 웹 접근성 측면에서 보자면,
alt의 설명이 좀 더 직관적이었으면 좋겠습니다. '즐겨찾기 버튼'이라던가, 실제 사용자의 입장에서 이해가 되게 작성해보면 어떨까요?
There was a problem hiding this comment.
alt 속성을 실제로 사용하게 될 사용자 입장에서 생각해보면 내용이 부족했던 것 같습니다.
혹시, alt 속성을 자세히 적어서 텍스트가 길어져도 상관 없을까요?
|
안녕하세요, 카일. 리뷰 반영이 늦어서 죄송합니다.🙇♂️ 개선
export function getRestaurantStorage() {
if (!localStorage.getItem("restaurant")) {
localStorage.setItem("restaurant", JSON.stringify(LIST_ITEM_CONTENTS));
}
return JSON.parse(localStorage.getItem("restaurant"));
}
export function setRestaurantStorage(restaurantInformation) {
localStorage.setItem("restaurant", JSON.stringify(restaurantInformation));
}=> RestaurantRepository라는 하나의 Restaurant 정보 저장 모듈을 만들어 기능을 관리하였습니다. const STORAGE_KEY = "restaurant";
export const RestaurantRepository = {
getAll: function () {
const savedData = localStorage.getItem(STORAGE_KEY);
if (savedData) {
return JSON.parse(savedData);
}
return;
},
save: function (restaurantInformation) {
localStorage.setItem(STORAGE_KEY, JSON.stringify(restaurantInformation));
},
};
export default RestaurantRepository;답변
질문
|
3 participants
안녕하세요, 카일
이번 리뷰도 잘 부탁드립니다!
리팩토링할 부분이 많았지만, 시간이 부족하여 우선 동작만 가능하도록 구현했습니다. 죄송합니다.🙇♂️
피드백 주시면 내용을 포함하여 리팩토링 진행하겠습니다!!
학습 목표
이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.
제출 전 체크 리스트
리뷰 요청 & 논의하고 싶은 내용
1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점
2) 이번 리뷰를 통해 논의하고 싶은 부분
step-1에서 step-2로 기능을 확장하는 과정에서 사이드 이펙트가 많이 발생했습니다. 이로 인해 컴포넌트에 전달되는 인자가 늘어나고 내부 로직이 복잡해져 오히려 재사용하기 어려운 컴포넌트가 되어버렸습니다. 이러한 사이드 이펙트를 줄이고 재사용성을 높이기 위해서는 어떤 방식으로 개선해 나가야 할까요?
업데이트가 발생할 때마다 ListController를 다시 호출하여 컨테이너의 innerHTML을 초기화하고 새로운 리스트 요소를 생성하여 DOM에 다시 추가하는 방식으로 재렌더링 효과를 구현했는데, 해당 방법은 변경된 매장 정보만 리렌더링 되는 것이 아니라 전체 매장 정보가 다시 렌더링되어 과하고 생각합니다. 현재 코드에서 어떤 방향으로 개선해야 업데이트된 매장만 리렌더링 시킬 수 있을까요?
✅ 리뷰어 체크 포인트
1단계
2단계