[2단계 - 음식점 목록] 윤생(이윤성) 미션 제출합니다. #53

2yunseong · 2023-03-09T13:08:50Z

안녕하세요 유조! 윤생입니다!!

날이 따뜻해지고 있어요~ 환절기인만큼 감기 조심하시길 바랍니다.

2단계는 기능 요구사항과 함께 이번 2단계는 다음과 같이 구현해보았어요!

먼저 태그가 상태를 가지고 있어야 편리한 것들이 몇개 있었어요. 특히 음식점 목록(ListItem)같은 경우는 각각을 눌렀을 때 “어떤 음식점을 눌렀는지 식별 가능해야 되어서, 편리성을 위해 dataset을 도입해보았습니다.
링크가 유효한지 검증하려는 기능을 구현하기 위해 html의 pattern 속성을 사용해보았습니다. 사용한 이유는 편리함도 있기 때문에 채택했고, 걸렸던 문제중에서는 pattern은 자바스크립트로 우회가 가능할 수도 있어 보안의 목적으로는 사용하면 안된다고 하지만, 지금 입력받는 링크는 보안의 목적으로 검사하는 건 아니라고 판단해 (http, https:// 로 시작하는지만 검사) 도입하게 되었습니다.
불필요한 setState를 삭제했습니다. 생각해보니 자신의 상태를 변경할 필요가 없는 단순히 위의 인자를 받아 보여주는 컴포넌트들은 setState를 작성해놓는거 자체가 혼란을 줄 수 있겠다고 생각했습니다.
이벤트 등록 또한 한 메서드로 묶었는데요,(addEvent) 이전에는 render안에서 전부 하다보니 코드가 불편하게 많이 늘어나는 것도 render의 가독성을 해치는 것 같다고 생각했고, 이벤트를 등록하는 일은 같은 일이라 생각해 다음과 같이 분리하였습니다.
이벤트 핸들러 네이밍을. on~ 하는 식으로 일관적으로 변경하였습니다. 핸들러 형식을 통일시키니 하위에서 사용하는 것의 목적이 더 들어났다고 생각하는데 유조의 생각도 자유롭게 들려주세요.
확실히 e2e 테스트와 단위테스트를 작성해놓고 리팩터링을 진행하니 직접 확인을 하지 않더라도 자동으로 확인할 수 있으니 매우 편리하였습니다~! e2e 테스트 같은 경우는 어떤 대상에 어떤 액션을 취하면 뭘 할 수있는지 위주로 작성해보았습니다.

아쉬운 점

일단은 재사용 컴포넌트 분리는 DropDown만 하였습니다. (이것도 1단계 막바지에서 했습니다) 이번에 Drawer를 재사용 하려 했는데.. 일단 구현을 하고 해야 낫겠다고 판단해 먼저 구현을 하고 재사용 분리를 고민해보려 하였습니다. (결론은 안했다는 변명입니다.) PR 이후에는 Drawer를 쪼개 보아 재사용 가능할 수 있게 사용할 수 있는지 고민해볼 예정입니다. (너무 늦게 리뷰를 받으면 오히려 유조의 피드백을 반영하지 못할 것 같아 미리 PR을 보냅니다.)
보시다시피.. RestaurantListPage가 거의 앱의 모든 상태를 가지고 있습니다. 거의 모든 컴포넌트가 이곳에 의존을 하죠. 분리를 해보려 이것 저것 노력은 해보았으나 커밋으로는 남기지 못하고 .. 그나마 반영해본 건 음식점 상세 정보 (RestaurantInfoDrawer) 을 띄울 때 상위로부터 데이터를 받지 않고 id 정보만 받아 직접 localStorage에서 꺼내오는 식으로 구현하였습니다.

이상입니다! 감사합니다 :D

배포 링크 : https://2yunseong.github.io/javascript-lunch/

- render와 setState를 강제하지 않음

yujo11

안녕하세요 윤생~~

dataset의 도입이나 pattern 속성을 사용하는 점에서 윤생이 열심히 학습하고 배운 내용을 적용하기 위해 노력하고 있다는게 느껴지네요 멋집니다~~ 👍 👍 👍

이벤트 핸들러 네이밍을. on~ 하는 식으로 일관적으로 변경하였습니다. 핸들러 형식을 통일시키니 하위에서 사용하는 것의 목적이 더 들어났다고 생각하는데 유조의 생각도 자유롭게 들려주세요.

좋은 컨벤션 같아요 전체 코드에서 핸들러가 일관성 있게 on- prefix를 가지게 되니 식별하기 편하네요~

확실히 e2e 테스트와 단위테스트를 작성해놓고 리팩터링을 진행하니 직접 확인을 하지 않더라도 자동으로 확인할 수 있으니 매우 편리하였습니다~! e2e 테스트 같은 경우는 어떤 대상에 어떤 액션을 취하면 뭘 할 수있는지 위주로 작성해보았습니다.

e2e 테스트를 작성할 때 BDD라는 기법도 많이 언급 되는데요. BDD에 대해서 찾아보시면 더 좋은 테스트 코드를 작성하는데 도움이 되실거 같아요!

추가로 코멘트 남기고 싶은 부분이 있는데요. 윤생도 아쉬운 점으로 RestaurantListPage가 거의 모든 상태를 가지고 있는걸 언급해주셨는데요.

코드레벨에서도 아쉬울 수 있겠지만 윤생이 개발한 애플리케이션을 사용하는 사용자의 사용성 측면에서도 아쉬움이 느껴졌어요.

이 gif를 보면 카테고리를 선택할 때마다 화면이 깜박이는걸 확인할 수 있는데요. 이러한 동작은 사용자 경험 측면에서 정말 좋지 않다고 생각해요.

SPA가 등장한 이유 중 가장 큰 것이 사용자 경험을 개선하기 위해서라고 생각하는데요. 현재 윤생의 애플리케이션은 CSR로 렌더되고 있지만 필요한 부분만 동적으로 업데이트하는 방식이 아니라 유저의 인터랙션이 발생할 때마다 앱 전체가 새로고침 되고 있기 떄문에 SPA라고 부르기는 어려울거라고 생각해요.

이런 부분을 중점적으로 신경써서 개선해보시는 것도 좋은 경험이 될거 같네요!

미션 진행하느라 고생 많으셨습니다 윤생! 👍 👍

yujo11 · 2023-03-10T10:32:03Z

cypress/e2e/spec.cy.js

+
+const TEST_URL = 'http://localhost:8080/';
+
+describe('음식점 추가 창', () => {


cypress로 e2e 테스트를 작성하셨군요 ! 👍 👍 👍

다시보니 파일 이름이 spec.cy.js네요 더 좋은 이름은 없을까요?

app이니 app.cy.js 가 좋아보입니다~! cypress에서 기본값으로 주어서 관례적으로 사용하는 줄 알았는데 그런건 아니였군요!!

yujo11 · 2023-03-10T10:32:28Z

cypress/e2e/spec.cy.js

+
+describe('음식점 추가 창', () => {
+  it('음식점 추가버튼(우측 상단)을 클릭하면 음식점 추가창을 볼 수 있다.', () => {
+    cy.visit(TEST_URL);


cy.visit()이 반복되는데 반복되는 코드를 줄일 수는 없을까요?

저번에 이러한 코드를 beforeEach로 줄여보았는데 한번 적용해보도록 하겠습니다!

yujo11 · 2023-03-10T10:42:11Z

cypress/e2e/spec.cy.js

+  });
+});
+
+describe('음식점 상세 정보 창', () => {


describe별로 파일을 나눠보시는 것도 좋을거 같네요

(가벼운 의견이니 적용하지 않으셔도 됩니다!)

yujo11 · 2023-03-10T10:58:41Z

src/type.ts

@@ -8,9 +9,11 @@ export type SelectOption = {
 };

 export type Restaurant = {
+  id: number;


👍👍👍

yujo11 · 2023-03-10T10:59:09Z

src/utils/api.js

@@ -0,0 +1,49 @@
+import { REQUEST_RASTAURANT_KEY } from './constants';
+
+export const fetchFavoriteId = (id) => {


localStorage를 API처럼 사용하는 아이디어가 좋네요 👍 👍 👍

yujo11 · 2023-03-10T11:00:24Z

src/pages/RestaurantListPage/index.ts

+
+  onOpenInfoDrawer(e: Event) {
+    const currentTarget = e.currentTarget as HTMLElement;
+    const rid = Number(currentTarget.dataset.restaurantId ?? '0');


rid는 RestaurantId의 줄임말인가요? 처음 식별자를 보고 이게 뭐지? 했는데요

짧은 식별자를 사용하는게 프로그래밍을 하는 단계에서는 편할 수 있어도 다른 사람이 볼 때는 어떤 의미인지 파악하기가 어려울 수 있어요. 이런 점도 같이 고려해보시면 좋을거 같네용

yujo11 · 2023-03-10T11:08:05Z

src/components/TabBar.ts

+  addEvent() {
+    const buttons = this.$target.querySelectorAll('.tab-bar-select');
+    for (const button of buttons) {
+      button.addEventListener('click', this.state.onClickTabBar);
+    }
+  }


DOM에서 가져온 요소들을 순회하면서 Event를 추가하는 것 말고 다른 방법은 없을까요?

이벤트 위임에 대해서 알아보시면 좋을거 같네요!

추가로 현재 메서드들에 따로 접근 제어자가 붙어있지 않은데요.

접근 제어자를 사용하게 되면 어떤 method가 외부에서 호출 될 수 있는지를 한 눈에 볼 수 있어 코드 읽기가 더욱 수월할거 같네요!

그 부분은 공부가 안되어서 이벤트 위임과 접근 제어자를 공부해보고 한번 적용해보도록 하겠습니다~! 고민 거리 던져주셔서 감사합니다 :D

yujo11 · 2023-03-10T11:11:47Z

src/components/TabBar.ts

+    this.$target = document.createElement('div');
+    this.$target.classList.add('tab-bar');
+    this.state = { tabBarSelect, onClickTabBar };
+
+    $parent.appendChild(this.$target);


render 메서드에서는 HTML String을 사용하고 있는데 constructor에서는 createElement를 활용하고 계시네요.

사용하신 기준이 있을까요?

따로 기준은 없었고, 단지 부모로 부터 받은 DOM에 $target을 등록해두고, 등록 되어있는 target안의 요소를 손쉽게 조작하기 위해 innerHTML을 사용했습니다.
HTML string을 사용한 이유는 innerHTML로 target을 조작하기 편리하여 사용하였습니다.

2yunseong · 2023-03-13T08:25:39Z

안녕하세요 유조!
날이 따뜻해지기 무섭게 꽃샘추위가 찾아왔네요 ㅠㅠ

피드백 주셨던 화면이 깜빡이는 문제에서는, GNB 헤더가 Page에 속해있을 이유가 없었는데 속해있으면서 음식점 배열 상태가 변경될 때 마다 새로 호출되었어요. 이 부분만 개선했고, 나머지 부분은 고민하다가 마감시간이 다가왔네요!ㅠㅠ

일단은 반영한 곳까지는 제출합니다! 컴포넌트 분리와 유조가 던져주신 SPA에 대해서는 아쉬움이 남지만요..!

다음은 지극히 개인적인 질문입니다!

컴포넌트 분리도 잘 해내지 못해 아쉬운 생각이 드는데, 이런 아쉬운 마음을 담아 스스로 이번 미션에 대한 회고를 어떻게하면 좋을지도 고민이 있습니다. 어떻게 회고를 해야 좋은 회고일까요? 물론 우테코에서도 회고에 대해 한 번 강의를 했었지만, 유조는 어떻게 생각하시는 지 궁금합니다. 다양한 의견을 듣는게 저 스스로에게 도움이 많이 될 것 같아 질문 드립니다!!

2주간 신경써주셔서 감사합니다!! :D

yujo11

안녕하세요 윤생~

벌써 마지막 리뷰군요~

피드백을 적용하는 과정들이 쉽지 않았을텐데 최선을 다해서 끝까지 구현하려고 노력하신 모습이 멋집니다 👍 👍 👍

컴포넌트 분리도 잘 해내지 못해 아쉬운 생각이 드는데, 이런 아쉬운 마음을 담아 스스로 이번 미션에 대한 회고를 어떻게하면 좋을지도 고민이 있습니다. 어떻게 회고를 해야 좋은 회고일까요? 물론 우테코에서도 회고에 대해 한 번 강의를 했었지만, 유조는 어떻게 생각하시는 지 궁금합니다. 다양한 의견을 듣는게 저 스스로에게 도움이 많이 될 것 같아 질문 드립니다!!

미션을 마치고 회고를 하려는건 정말 좋은 시도인데요! 👍 👍

저는 Keep/Problem/Try 를 나눠서 회고를 작성하는 편이에요

Keep에는 잘하고 있는 점. 계속 했으면 좋겠다 싶은 점
Problem에는 문제가 있다고 생각 되는 점, 변화가 필요한 점
Try에는 잘하고 있는 점을 더 잘 하기 위해서나 문제가 있다고 생각되는 점을 해결하기 위해 시도해 볼 점

이런 식으로 회고를 해보는데요. 이러한 회고 템플릿을 통해서 회고를 진행해보셔도 좋을거 같아요!

여기서 한가지만 더 얘기하면 Try에 적은 일들을 실제로 수행하기 위해 구체적은 Action Plan을 적어보는 것도 추천드리고 싶어요!

ex) Try - 컴포넌트를 더 잘 분리해야 한다. -> Action Plan - Bottom-Up 방식으로 필요한 컴포넌트가 무엇이 있는지 파악하고 Element 단위의 컴포넌트를 먼저 만든다.

미션 진행하느라 고생 많으셨습니다 윤생! 이제 하나 남은 레벨 1 마지막 미션도 화이팅입니다!

앞으로의 여정도 응원하겠습니다!! 👍 👍 👍

2yunseong added 30 commits March 8, 2023 15:07

docs: 요구사항 목록 갱신

48e1f38

refactor: Component Interface 변경

dfe8c69

- render와 setState를 강제하지 않음

feat: 탭바 UI 토글 기능 추가

d46d6a0

feat: 탭 바 메뉴별 정렬 로직 구현

7a8e9a2

feat: 자주 가는 음식점 버튼 토글 기능 구현 및 저장소에 요청

a4d4ec4

fix: 좋아요 상태 변경 시 리렌더링 로직 추가

e888f73

feat: Drawer 좋아요 토글 동작 구현

32531f0

refactor: 이벤트 리스너 등록 로직 메서드로 묶음

07e7399

refactor: export default 축약

9b57870

refactor: 사용성 개선

78e877d

docs: 요구사항 최신화 및 e2e테스트 기능 명세 작성

481bc98

test: 음식점 추가 버튼 클릭 테스트

7bf79aa

test: 음식점 추가 창 취소하기 버튼 테스트

92c8d10

test: 음식 종류 별 정렬 기능 테스트

e98102d

refactor: 의미에 맞지 않는 html 태그 변경 및 class name 할당

aa9f6aa

test: 음식점 이름/거리 순 정렬 기능 테스트

f19afb9

test: 탭바 테스트

e068708

test: 음식정 상세 정보 창 테스트

aaa83f4

refactor: 이벤트 핸들러 이름 일관성 부여

d4af145

refactor: 불필요한 setState 제거

4d1c79a

refactor: 컴포넌트에 불필한 로직 분리

fd53765

refactor: 메서드 분리

c72bde7

refactor: 비구조화 할당 제거

1985331

refactor: 디렉터리 구조 변경

65b1a56

test: 음식점 목록 자주가는 음식점 토글 기능 테스트

734535a

docs: 요구사항 목록 최신화

523f93e

feat: 고유한 아이디를 생성하게 구현

944a231

refactor: 레스토랑 등록 로직 분리

9d2bb91

refactor: 사용하지 않는 import 제거

3e41b7a

refactor: 재사용 되는 로직 분리

0b92bc4

refactor: img src 소스 링크 상수화

2bea033

yujo11 self-requested a review March 9, 2023 23:10

yujo11 self-assigned this Mar 9, 2023

yujo11 requested changes Mar 10, 2023

View reviewed changes

2yunseong added 9 commits March 10, 2023 22:20

test: 코드 중복 제거

aea911b

test: 테스트 이름 변경

0188bdd

refactor: 축약된 이름 변경

44c14bf

refactor: 불필요한 리렌더링 제거

00abc75

refactor: 이벤트 위임으로 핸들러 일반화

f5c12a8

refactor: 이벤트 위임으로 변경

a3fa9b1

refactor: DropDown 이벤트 핸들러 메서드 로직 병합

8b8d4a0

refactor: 접근 제한자 부여

959dc93

refactor: drawer html 분리

b6a6485

yujo11 approved these changes Mar 13, 2023

View reviewed changes

yujo11 merged commit e299419 into woowacourse:2yunseong Mar 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 음식점 목록] 윤생(이윤성) 미션 제출합니다. #53

[2단계 - 음식점 목록] 윤생(이윤성) 미션 제출합니다. #53

2yunseong commented Mar 9, 2023 •

edited

Loading

yujo11 left a comment

yujo11 Mar 10, 2023

yujo11 Mar 10, 2023

2yunseong Mar 10, 2023

yujo11 Mar 10, 2023

2yunseong Mar 10, 2023

yujo11 Mar 10, 2023

yujo11 Mar 10, 2023

yujo11 Mar 10, 2023

yujo11 Mar 10, 2023

yujo11 Mar 10, 2023

yujo11 Mar 10, 2023

2yunseong Mar 10, 2023

yujo11 Mar 10, 2023

2yunseong Mar 10, 2023

2yunseong commented Mar 13, 2023

yujo11 left a comment


		const TEST_URL = 'http://localhost:8080/';

		describe('음식점 추가 창', () => {

		@@ -0,0 +1,49 @@
		import { REQUEST_RASTAURANT_KEY } from './constants';

		export const fetchFavoriteId = (id) => {

[2단계 - 음식점 목록] 윤생(이윤성) 미션 제출합니다. #53

[2단계 - 음식점 목록] 윤생(이윤성) 미션 제출합니다. #53

Conversation

2yunseong commented Mar 9, 2023 • edited Loading

아쉬운 점

yujo11 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2yunseong commented Mar 13, 2023

yujo11 left a comment

Choose a reason for hiding this comment

2yunseong commented Mar 9, 2023 •

edited

Loading