표기단계 조절 및 테마선택 상태 관리를 redux로 변경 #166

gitgitWi · 2020-12-10T15:38:58Z

구현 내용

표기단계 조절 및 테마선택 상태 관리를 redux로 변경

상태 관리를 useState -> react-redux로 변경 (fd86630)
- action/reducer 추가
- 기존 hooks 수정
layer 변경에 따른 수정 사항 적용
- transit 레이어 위치 변경에 따른 import 경로 수정 (0e6b75d)
- 행정구역 중 locality layer id 수정 (6a9da8f)
  - AdministrativeNameType에는 locality로 지정되어 있으나,
  - layerId는 settlement로 되어 있어 오류 발생
  - settlement -> locality로 id 수정

에러 해결 내용

행정구역 표기단계 조절시 에러 및 전체 스타일 조정 에러 해결 (`7794662`)

이전 커밋 (fd86630)에서 행정구역 중 locality 속성 관련 문제 해결
- Element로 labelText 만 갖고 있어 다른 속성 적용함
세부 스타일 설정하고 표기단계 조절하는 경우, 전체 스타일 초기화 되는 문제 해결
- changeStyle -> replaceStyle로 함수 변경하고
- useWholeStyle에서 전체 저장된 스타일 가져오는 함수 getWholeStyle 생성
- 표기단계 조절시 기존 스타일 spread 연산으로 추가하여 적용함

TODO

표기단계 조절에 따른 변경사항은 isChecked로 반영되지 않는데, 초기 환경 설정부분이라 괜찮을지??
- replaceStyle 함수
표기단계 조절 및 테마 변경사항 log에 저장?
- log 저장 타입 변경해야 함
리팩토링

…dux로 변경 - 상태 관리를 useState -> react-redux로 변경 - action/reducer 추가 - 기존 hooks 수정

- `AdministrativeNameType`에는 locality로 지정되어 있으나, - layerId는 settlement로 되어 있어 오류 발생 -settlement -> locality로 id 수정

… 조정 에러 해결 - 이전 커밋 (fd86630)에서 행정구역 중 `locality` 속성 관련 문제 해결 - Element로 labelText 만 갖고 있어 다른 속성 적용함 - 세부 스타일 설정하고 표기단계 조절하는 경우, 전체 스타일 초기화 되는 문제 해결 - `changeStyle` -> `replaceStyle`로 함수 변경하고 - `useWholeStyle`에서 전체 저장된 스타일 가져오는 함수 `getWholeStyle` 생성 - 표기단계 조절시 기존 스타일 iterate로 추가하여 적용함

ghost · 2020-12-11T01:49:01Z

src/hooks/common/useWholeStyle.ts

@@ -60,6 +61,18 @@ function useWholeStyle(): WholeStyleHook {
    setFlag(false);
  }, [flag]);

+  const getWholeStyle = () => {


store에서 관리되는 상태들은 역할/용도에 맞게 묶거나 그룹화하면 상태관리해도 좋을거 같습니다!

이러면 전체 상태의 구조를 파악하기 쉽고, 각 역할 별로 상태를 나눠 관리/유지보수하는데 도움이 될거같아요~

아래 코드처럼 사용 될 경우(모두 state 아래 동일 depth로 표현할경우), 이 역할에 대한 추론이 어렵고(UI인지/Layer인지 등),

다른 역할을 하는 상태에서 동일한 state명을 사용할 경우 충돌 할 수도 있을거같아요~

그리고 이 코드(const getWholeStyle = () => {)를 포함하여, 아래 코드의 중복이 제거되는 이점도 있겠네요~

const { poi, landscape, administrative, road, transit, water, marker, } = useSelector<RootState>((state) => state) as WholeStyleStoreType;

ex)

const { layers } = useSelector<RootState>((state) => state.layers) as WholeStyleStoreType;

말씀하신 것처럼 store 리팩토링이 필요할 것 같습니다!

getWholeStyle 함수처럼 단순한 로직도 코드가 길어지는 문제도 있고,

모든 reducer가 하나의 store에 동일하게 위치하다보니,
단순히 표기단계 조절하는 reducer 사용하는데도 map, layers, sidebar 등 모든 reducer를 탐색하고 있더라구요

#107 과 함께 팀원들과 논의해서 리팩토링 해보도록 하겠습니다

Eunsol0410

정훈님 코드 리뷰하다가 제가 리팩토링을 살짝했어요 ㅎㅎ 부족한 부분은 나중에 더 채워주세요!

Eunsol0410 · 2020-12-11T13:31:00Z

src/store/depth-theme/action.ts

+  roadDepth?: number;
+  administrativeDepth?: number;
+  themeIdx?: number;
+}


두 액션의 타입을 분리하는 게 좋지 않을까요?!

Eunsol0410 · 2020-12-11T13:42:57Z

src/store/depth-theme/reducer.ts

+      return {
+        ...state,
+        ...changedDepth,
+      };


payload에 selectedFeature, selectedDepth로 받으시는 이유가 있을까요? 바로 roadDepth/administrativeDepth를 업데이트 해도 될 것 같은데!
그리고 전체 상태에는 selectedFeature, selectedDepth를 저장하지 않는 것 같은데 그럼 상태에서 빼도 좋을 것 같아요

Eunsol0410 · 2020-12-11T13:46:50Z

src/hooks/sidebar/useSidebarDepthItem.ts

-    Array.from(Array(high - low).keys()).map((num) => num + low),
-  ];
+  const getDepthRange = () =>
+    Array.from(Array(high - low).keys()).map((num) => num + low);


map을 돌면 배열이 리턴되어서, Array.from은 빼도 될 것 같아요~

Eunsol0410 · 2020-12-11T13:50:15Z

src/hooks/sidebar/useSidebarDepthItem.ts

  const depthRef = useRef<HTMLInputElement>(null);
  const map = useSelector<RootState>((state) => state.map.map) as mapboxgl.Map;
+  const { getWholeStyle, replaceStyle } = useWholeStyle();

  const depthRangeHandler = () => {
    const changedDepth = Number(depthRef.current?.value);


은식님도 전에 리뷰를 남겼던 것 같은데... useRef를 사용하는 것보다 이벤트 핸들러에 전달된 이벤트 객체를 사용하는 게 좋을 것 같아요~

Eunsol0410 · 2020-12-11T14:25:10Z

여기 함수도 좀 큰 것 같아서 Hook 위에 부분은 따로 분리해도 좋을 것 같아요~

Eunsol0410 · 2020-12-13T08:35:28Z

src/hooks/sidebar/useSidebarDepthItem.ts

+        visibility,
+        currentStyleState
+      );
+      replaceStyle(changedStyle);


layer를 직접 여기서 수정하면 안될 것 같아요. replaceStyle을 실행하면 거기서도 레이어의 속성을 전부 변경하게 되지 않을까요?!

gitgitwi added 4 commits December 10, 2020 17:32

[FIX] transit 레이어 위치 변경에 따른 import 경로 수정

0e6b75d

[FEAT] [boostcamp-2020#64,boostcamp-2020#66] 표기단계 조절 및 테마선택 상태 관리를 re…

fd86630

…dux로 변경 - 상태 관리를 useState -> react-redux로 변경 - action/reducer 추가 - 기존 hooks 수정

[FEAT] [boostcamp-2020#68] 행정구역 중 locality layer id 수정

6a9da8f

- `AdministrativeNameType`에는 locality로 지정되어 있으나, - layerId는 settlement로 되어 있어 오류 발생 -settlement -> locality로 id 수정

gitgitWi self-assigned this Dec 10, 2020

gitgitWi requested review from qkrdmstlr3, yyjjjj, Eunsol0410, limlimlimlimlimlim, a user and GodDrinkTeJAVA December 10, 2020 15:39

gitgitWi added bug Something isn't working must This label is for issue which should be implemented 스타일 유형 labels Dec 10, 2020

gitgitWi added this to the 4주차 milestone Dec 10, 2020

This was linked to issues Dec 10, 2020

Fewer Options - 표기 단계 조절(도로) #64

Closed

Fewer Options - 표기 단계 조절(행정구역) #66

Closed

gitgitWi merged commit 1a39f1a into boostcamp-2020:feature/history Dec 11, 2020

ghost reviewed Dec 11, 2020

View reviewed changes

Eunsol0410 reviewed Dec 11, 2020

View reviewed changes

Eunsol0410 reviewed Dec 13, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

표기단계 조절 및 테마선택 상태 관리를 redux로 변경 #166

표기단계 조절 및 테마선택 상태 관리를 redux로 변경 #166

gitgitWi commented Dec 10, 2020 •

edited

Loading

ghost Dec 11, 2020

gitgitWi Dec 11, 2020

Eunsol0410 left a comment

Eunsol0410 Dec 11, 2020

Eunsol0410 Dec 11, 2020

Eunsol0410 Dec 11, 2020

Eunsol0410 Dec 11, 2020

Eunsol0410 commented Dec 11, 2020

Eunsol0410 Dec 13, 2020

표기단계 조절 및 테마선택 상태 관리를 redux로 변경 #166

표기단계 조절 및 테마선택 상태 관리를 redux로 변경 #166

Conversation

gitgitWi commented Dec 10, 2020 • edited Loading

구현 내용