-
Notifications
You must be signed in to change notification settings - Fork 89
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
[2단계 - 자주 가는 음식점] 해리(최현웅) 미션 제출합니다. #155
Changes from 25 commits
6f0d55a
13e87fa
5eb7e3e
ec3f055
72a62ee
c09a809
db3dea7
46653ca
41f1b08
a3b60ad
6d9f200
ecadfc6
b7329e5
188b413
f9f8200
77b8110
12ebc28
2d2b05e
5e184f6
dbd4317
794955c
4774765
6651777
d0caaf6
5435219
dafd4c6
75ea25d
e37988e
a1e1a89
003fa7c
c0158c9
0279431
8ecc48e
e52abc1
842dc67
f634e1c
05cbb0a
6660602
2660380
a695f12
b7852b8
bcb84ad
af74fab
b14dedd
b4ca733
eaf0992
d73df87
309dcca
fb86be6
c73d982
cd60e5f
97ceeea
e58c478
7501413
3fb961a
c3612a2
2f2cf84
323add7
8ec47cf
17691a0
42d0917
9e4b00f
1cb1d02
4d0c2f3
4599c6c
c701028
c5ade21
2bebec1
191f542
c818b2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
export { default as addButton } from "../../templates/add-button.png"; | ||
export { default as categoryKorean } from "../../templates/category-korean.png"; | ||
export { default as categoryAsian } from "../../templates/category-asian.png"; | ||
export { default as categoryChinese } from "../../templates/category-chinese.png"; | ||
export { default as categoryEtc } from "../../templates/category-etc.png"; | ||
export { default as categoryJapanese } from "../../templates/category-japanese.png"; | ||
export { default as categoryKorean } from "../../templates/category-korean.png"; | ||
export { default as categoryChinese } from "../../templates/category-chinese.png"; | ||
export { default as categoryWestern } from "../../templates/category-western.png"; | ||
export { default as categoryEtc } from "../../templates/category-etc.png"; | ||
export { default as linedFavoriteIcon } from "../../templates/favorite-icon-lined.png"; | ||
export { default as filledFavoriteIcon } from "../../templates/favorite-icon-filled.png"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import BaseComponent from "../BaseComponent"; | ||
import { CATEGORIES } from "../../constants/menu"; | ||
import { CategoryStringWithoutAll } from "../../types/menu"; | ||
import { | ||
categoryKorean, | ||
categoryAsian, | ||
categoryChinese, | ||
categoryJapanese, | ||
categoryWestern, | ||
categoryEtc, | ||
} from "../../assets"; | ||
|
||
class CategoryImage extends BaseComponent { | ||
private isCategoryType(category: string): category is CategoryStringWithoutAll { | ||
return ["한식", "아시안", "일식", "중식", "양식", "기타"].includes(category); | ||
} | ||
|
||
private categoryToImg(category: CategoryStringWithoutAll) { | ||
if (!category) return; | ||
|
||
switch (category) { | ||
case CATEGORIES.korean: | ||
return categoryKorean; | ||
case CATEGORIES.asian: | ||
return categoryAsian; | ||
case CATEGORIES.japanese: | ||
return categoryJapanese; | ||
case CATEGORIES.chinese: | ||
return categoryChinese; | ||
case CATEGORIES.western: | ||
return categoryWestern; | ||
case CATEGORIES.etc: | ||
return categoryEtc; | ||
default: | ||
break; | ||
} | ||
} | ||
|
||
render() { | ||
const category = this.getAttribute("category") ?? ""; | ||
if (!this.isCategoryType(category)) return; | ||
|
||
const categoryImage = this.categoryToImg(category); | ||
this.innerHTML = /*html*/ ` | ||
<div class="restaurant__category"> | ||
<img src=${categoryImage} alt=${category} class="category-icon"> | ||
</div> | ||
`; | ||
} | ||
} | ||
|
||
customElements.define("category-image", CategoryImage); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,43 @@ | ||||||
import BaseComponent from "../BaseComponent"; | ||||||
import { MENU_APP_EVENTS } from "../../constants/event"; | ||||||
import { toggleFavoriteStateByName } from "../../domains/Restaurants"; | ||||||
import { filledFavoriteIcon, linedFavoriteIcon } from "../../assets/index"; | ||||||
|
||||||
class FavoriteIcon extends BaseComponent { | ||||||
protected render() { | ||||||
const isFavorite = this.getAttribute("is-favorite") === "true" ? true : false; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 간결하게 줄일 수 있을 것 같아요 :)
Suggested change
|
||||||
|
||||||
this.innerHTML = /*html*/ ` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 주석은 특별한 용도가 있나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오 저도 설치했습니다 감사합니다 :) |
||||||
<img alt="favorite-icon" class="favorite-icon" | ||||||
src=${isFavorite ? filledFavoriteIcon : linedFavoriteIcon}></img> | ||||||
`; | ||||||
} | ||||||
|
||||||
private toggleFavoriteIcon(iconElement: HTMLImageElement) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. img.src 값으로 지정된 순간 filled 이미지가 로드되고 있어, 이 시점을 앞당겨주는 방향으로 말씀드려보았습니다. filled 이미지를
등의 방법이 있을 것 같아요 :) |
||||||
const prevIcon = iconElement?.getAttribute("src"); | ||||||
iconElement?.setAttribute("src", prevIcon === linedFavoriteIcon ? filledFavoriteIcon : linedFavoriteIcon); | ||||||
} | ||||||
|
||||||
private toggleFavoriteState() { | ||||||
const restaurantName = this.getAttribute("restaurant-name") ?? ""; | ||||||
toggleFavoriteStateByName(restaurantName); | ||||||
} | ||||||
|
||||||
private handleToggleFavoriteIcon(element: HTMLElement) { | ||||||
const iconElement = element.closest("img"); | ||||||
if (!(iconElement instanceof HTMLImageElement)) return; | ||||||
|
||||||
this.toggleFavoriteIcon(iconElement); | ||||||
this.toggleFavoriteState(); | ||||||
this.emitEvent(MENU_APP_EVENTS.renderRestaurants); | ||||||
} | ||||||
|
||||||
protected setEvent() { | ||||||
this.addEventListener("click", (event) => { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사용자와 상호작용하는 요소들은 HTML에 잘 정의가 되어있어요. (button, input, ... 등) 이 토글 기능의 경우, 버튼 또는 label 태그와 함께 input의 type="checkbox"로 구현하는 것이 좋아보입니다. 말 그대로 토글 기능이니까요 :) 시간 여유가 있다면 input의 type="checkbox"로 구현을 해주시면 좋겠습니다. 없다면 그렇구나 하고 넘어가주세요 :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아하 그렇군요...! img는 사용자와 상호작용하는 태그로 사용하기보다는 사용자에게 시각적 요소를 보여주는 태그로 사용하는게 맞겠네요. 하루를 통해서 정말 많은 팁들을 얻어가는 것 같아요! 감사해요 하루 🙇♂️ |
||||||
if (!(event.target instanceof HTMLElement)) return; | ||||||
this.handleToggleFavoriteIcon(event.target); | ||||||
}); | ||||||
} | ||||||
} | ||||||
|
||||||
customElements.define("favorite-icon", FavoriteIcon); |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import BaseComponent from "./BaseComponent"; | ||
import { MENU_APP_EVENTS } from "../constants/event"; | ||
import { initRestaurantStorage } from "../domains/Restaurants"; | ||
|
||
class MenuApp extends BaseComponent { | ||
constructor() { | ||
initRestaurantStorage(); | ||
super(); | ||
} | ||
|
||
render() { | ||
this.innerHTML = /*html*/ ` | ||
<app-header></app-header> | ||
<restaurant-tab-container class="flex justify-between tab-container"></restaurant-tab-container> | ||
<main> | ||
<option-selector-container></option-selector-container> | ||
<restaurant-list></restaurant-list> | ||
</main> | ||
<modal-wrapper | ||
open-type=${MENU_APP_EVENTS.openAddForm} | ||
id='add-form'> | ||
<restaurant-add-form></restaurant-add-form> | ||
</modal-wrapper> | ||
<modal-wrapper | ||
open-type=${MENU_APP_EVENTS.openRestaurantDetail} | ||
id="restaurant-detail" > | ||
<restaurant-detail></restaurant-detail> | ||
</modal-wrapper> | ||
`; | ||
} | ||
} | ||
|
||
customElements.define("menu-app", MenuApp); |
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.
새로운 음식 카테고리가 추가되면 여기만 누락될 여지가 있어보여요. 정의해둔
CATEGORIES
상수를 활용할 수 있을까요~?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.
저도 정의해 둔 상수를 활용하는 것을 고려해봤었는데,
이 방식으로 사용할 경우
Object.values
메서드가 반환하는 데이터의 타입이위 사진과 같이 추론되어 string을 인자로 받는
isCategoryType
메서드에서위와 같은 에러가 발생하는 것을 확인할 수 있었습니다.
type assertion
을 지양하는 것이 좋다고는 하지만 새로운 카테고리가 추가될 때 누락되는 상황이 발생하는것 보다type assertion
을 사용하더라도 변경에 유연하게 대처되는 코드가 더 좋을 것 같다는 생각이 드네요.Object.values
메서드를 활용하는 것으로 수정해보았습니다!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.
---- 추가 코멘트 ----
위와 같이 사용하니
type assertion
을 사용하지 않아도 돼서 제거했습니다! 추상화 방법 추천 감사드립니다. 🙇♂️