[Fix/#294] 우선순위 입력 페이지에서 가능시간 입력 페이지로 되돌아가면 우선순위 초기화

[ljh0608]

🌀 해당 이슈 번호

• close [Fix] 우선순위 입렵페이지에서 가능시간 입력페이지로 되돌아가면 우선순위 초기화 #294

🔹 어떤 것을 변경했나요?

• 온보딩에 햄버거버튼 사이드메뉴가 온보딩이미지보다 뒤에쌓이는 문제 해결
• x버튼이 메뉴처럼 스르륵나오는것이 아니라 fixed로 고정되어 있는 문제 해결
• 우선순위 입력 페이지에서 시간입력페이지로 (뒤로가기) 이동했을 때 우선순위 초기화
• 우선순위 드롭다운 입력순이 아닌 날짜/시간순으로 정렬
• 우선순위 페이지 중복코드 개선

🔹 어떻게 구현했나요?

[ 우선순위 드롭다운 날짜/시간순 정렬 ]
우선순위 드롭다운 정렬은 parsing함수를 만들어서 sort했습니다.
sort메소드는 해당 배열을 변경하는 것이라 불변성에 문제가 있을 것이라 예상해서 toSorted 메소드를 사용했습니다. 원본 배열을 정렬하려했는데 그렇게하면 배열과 객체를 변환할 때 key를 사용하는 부분에서 문제가 생깁니다. 따라서 보여주는 UI (드롭다운 부분)만 따로 정렬된 배열을 사용했고, 우선순위 선택은 key를 기준으로 짜여진 로직이라 문제 없었습니다.

• toSorted 메소드가 빌드상 에러가 나네요. 노드버전에 문제가 있어서 올렸는데 해결이 안돼서 일단 slice 메소드로 복사해서 사용했습니다! toSorted메소드 최신 메소드라 좋아보였는데 사용 못해서 아쉽네요!

[ 우선순위 입력페이지에서 뒤로가면 우선순위 초기화 ]
step을 state로 관리하고 있어서 useEffect로 step이 변경될 때 selectedSlots의 priority를 모두 0으로 초기화했습니다.
근데 매번 selectedSlots를 다룰때 object.entries메소드를 사용하고 다시 fromEntries메소드를 사용하는데 제가 잘못접근하고 있는게 있는게 있을지 말씀해주시면 감사하겠습니다.
또 저번에 말씀해주신 중첩객체를 사용하는 이유에 대해서도 다시 한번 의견 듣고싶어요!

🔹 PR 포인트를 알려주세요!

중복을 많이 줄였다!

🔹 스크린샷을 남겨주세요!

[ 우선순위 드롭다운 날짜/시간순 정렬 ]
https://github.com/user-attachments/assets/acf5658c-5c48-44cc-ac48-5e1c7e4bdbd5

[ 우선순위 입력페이지에서 뒤로가면 우선순위 초기화되도록 구현 ]
https://github.com/user-attachments/assets/1d2a5e4e-a98c-47e3-993e-40580e4bdaee

[ x버튼 따로노는거 수정, 메뉴가 온보딩 애니메이션보다 뒤에 뜨는 부분 개선 ]
https://github.com/user-attachments/assets/3a6816e4-b1e0-4f53-8365-d2a6bf3a5175

(흠... 영상이 잘안보이네요 왜그럴까요?)

[github-actions]

빌드에 실패했습니다.

[simeunseo]

많은 작업을 빠르게 해주셨네요... 멋집니다!!
리뷰가 좀 긴데 확인해보시고 같이 공부해보면 좋겠습니다! :D

[simeunseo]

z-index를 계속 하드코딩 하게 되면 앞으로도 모달, 바텀시트 등등 z-index가 중요한 상황에서 예상치 못한 결과가 생길 수 있으니
사이드바의 z-index를 늘리는 것보다, swiper에 기본적으로 적용되어 있는 z-index: 1을 없애는게 낫지 않을까?라는 생각이 들었어요.

그래서 swiper에 왜 z-index: 1이 적용되어 있는지를 좀 알아봤는데, 해당 swiper 라이브러리 코드를 보니 z-index 부분에 대해 Fix of Webkit flickering 이라는 주석이 써있더라구요. 관련 PR이 없어서 더 자세히 알아보지는 못했어요.
일단 제가 swiper에서 z-index를 0으로 맞추고 크롬과 사파리에서 모두 확인해보았으나 문제는 없었는데... swiper 개발자가 z-index를 넣은데는 이유가 있을테니까요..ㅎㅎ

그래서 결론은, 우선 이렇게 해두되 이번 스프린트 끝내고나서 z-index를 상수로 빼서 관리를 하는 등 위계를 잘 정리해보면 좋을 것 같습니다.
서비스 사이즈가 작다보니 z-index가 쓰이는 곳도 그리 많지 않은 것 같아서요. 헤더, 사이드바, 드롭다운, 모달, 바텀시트 등..? 각각에 맞게 z-index를 상수화하면 덜 복잡하게 관리할 수 있을 것 같습니다.

결론만 확인해주시면 될거같긴한데 제 생각 과정을 공유드려봤읍니다!

[ljh0608]

좋네요 z-index를 상수화한다는 생각은 못해봤는데 공부를 해봐야겠네요 !
이런식의 상수화를 말씀하시는게 맞으신가요??
완전 좋지만 그전에 필요없는 z-index사용한 부분을 개선해보면 좋겠네요!!

swiper의 z-index:1이 적용되어있는 이유는 Webkit의 깜빡임 문제때문이라고 하네요. webkit은 마크업을 기반으로 웹브라우저를 만드는데 기반을 제공하는 오픈소스 프레임워크라고 합니다. 저도 찾아보며 공부할 수 있어서 좋았습니다!

갑자기 궁금해서 z-index에 대해서도 조금 더 공부해보니

1. position: relative, absolute는 일반적인 요소보다 더 높은 쌓임맥락을 가집니다.
2. 자식의 z-index값은 부모요소 내부에서만 의미가 있습니다.
3. 같은 포지션, 같은 z-index일 경우 나중에 쓰여진 HTML 소스가 더 높이 위치합니다.

라고 이해하고 사용중입니다.

감사합니다~

[simeunseo]

넵 첨부해주신 링크에 잘 설명되어있네요!! 제가 처음에 생각한 방법은 아래처럼 단순한 상수화였는데, 어떤 방법이 적절할지는 저희 서비스내에 zIndex가 필요한 부분을 정리해보고 결정하면 좋을 것같아요. zIndex가 필요하지 않은데 쓰이고있는 것들이 있다면 정리하구요.

export zIndex = {
  Sidebar: 1,
  Modal: 2,
  ...

[simeunseo]

이 부분 회의때도 얘기했지만 다시 정리해서 의견 남기겠습니다!

1. useEffect의 사용 관련
   React 공식문서 중 You might not need a effect 문서를 참고했습니다. 문서의 첫단락에 보면 useEffect가 필요하지 않은 대표적인 경우 중 첫번째로 "렌더링을 위해 데이터를 변환하는 데 Effect가 필요하지 않습니다." 라고 언급하고 있습니다. 그리고 이 경우에 대한 예시로, props 또는 state에 따라 state 업데이트하기 부분과 props 변경시 모든 state 초기화 부분이 저희가 맞딱들인 문제와 상당히 유사하다고 느껴집니다. 저희도 scheduleStep이라는 state가 특정 값으로 변경되었을 때, selectedSlots라는 상태를 변경시키고 싶은 상황이니까요.
   이 문서들에서 해당 상황에 useEffect를 쓰지 말라고 하는 이유는, useEffect가 렌더링이 완료된 후 발생하기 때문입니다. 저희 상황에 적용하면, priority들이 초기화되지 않은 상태로 렌더링이 됐다가, useEffect를 통해 priority를 모두 0으로 만들고나서 시간표를 다시 렌더링해야하기 때문입니다. 비효율적이죠...

react developer tools로 직접 확인해보니 역시 렌더링이 두번 일어나고 있었습니다.
useEffect 사용 X(초기화 기능 X)

2024-08-04.3.31.32.mov

useEffect 사용 O(초기화 기능 O)

2024-08-04.3.31.56.mov

제 생각에 selectedSlots라는 하나의 state에서, (가능 시간 입력 페이지에서 사용되는) 선택된 날짜 데이터와 (우선순위 입력 페이지에서 사용되는) priority 데이터가 같이 관리되고 있다는 구조적인 문제에서 원인을 따져볼 수도 있을 것 같습니다. 가능 시간 입력 시간표와 우선순위 입력 시간표가 아예 다른 컴포넌트로 분리되어있다면, 가능 시간 입력 시간표에서는 priority값을 사용할 필요가 없고, 우선순위 입력 시간표가 렌더링 될 때 priority가 항상 0으로 초기화되게끔 한다면 해결이 될 수도 있지 않을까....
하지만 가능시간입력->우선순위입력이 유기적으로 연결되어있고 시간표 컴포넌트를 재사용하는 이상, 이 부분을 건드는건 오히려 문제가 더 커질 것 같습니다.

2. useEffect를 쓰지 않고 header에서 setState를 하는 방법
   그래서 useEffect를 쓰지 않는 방법으로 header에서 초기화 처리를 하는 방법을 제시해주셨습니다. 모든 페이지에 공통으로 삽입되는 헤더에 특정 페이지만을 위한 의존성을 삽입하는게 맞지 않다고 저희가 입을 모았었는데요. 제가 실제로 header에 넣는 방법으로 코드를 짜보니 일단 useEffect없이 해결이 되는 건 맞습니다. 그런데 header에서 초기화를 하려면 selectedSlots에 대한 context를 주입받아야하는데 그건 구조상 불가능합니다. 그래서 역시 이 방안은 기각입니다

3. 결론
   현재 이렇게 useEffect를 사용하는 로직이 불필요한 리렌더링을 일으키는 안티패턴임은 분명합니다. 그러나 이로인해 직접적인 문제가 발생하는 것은 아니며, 구조상 useEffect를 배제하는게 쉽지 않기 때문에 다음에 다시 고민해보는 것도 괜찮을 것 같다는 의견입니다.

[simeunseo]

추가로, entries메소드 사용으로 인한 번거로움을 언급해주셨는데 다음과 같이 작성하면 어떨까요?

 const resetPriorities = (selectedSlots: SelectedSlotType) => {
    const updatedSelectedSlots: SelectedSlotType = {};
    for (const key in selectedSlots) {
      updatedSelectedSlots[key] = {
        ...selectedSlots[key],
        priority: 0,
      };
    }
    setSelectedSlots(updatedSelectedSlots);
  };

  useEffect(
    () => {
      if (scheduleStep === 'selectTimeSlot') resetPriorities(selectedSlots);
    },
    [scheduleStep],
  );

추가로, useEffect 내부에서 실행할 작업이 명시적으로 드러나는 게 좋다고 생각해서 위와 같이 resetPriorities 함수를 따로 빼는것이 어떨까합니다.

[ljh0608]

말씀해주신 모든 의견이 좋아보입니다. 배열로 변경하지 않고도 충분히 set할수 있겠군요!
PR참고해서 useEffect 사용없이 시간입력 페이지에서 우선순위를 모두 초기화하는 방향으로 가고싶었는데, 결국 가능시간 입력페이지에서

 if (scheduleStep === 'selectTimeSlot') resetPriorities(selectedSlots);

이런 로직을 첫 렌더링때 사용하려면 결국 useEffect가 필요할 것 같군요.. header에 onClick함수를 전달하는 방향으로 header가 개선되는 방향이 가장 적합할 것 같다는 생각이 드는데 어떻게 생각하시나요??

[ljh0608]

useEffect 사용하지 않고 현재 상황에서 해결할 수 있는 방안을 찾았습니다.

저 역시 effect를 사용하는 것에 충분히 이질감을 느끼고 있었고 위와 같은 생각을 하며 추후 개선해야지.. 하고있다가 조금 더 고민해보면서 effect를 사용하지 않고 평소의 흐름에서 state를 초기화할 수 있는 경우의 수를 생각해봤어요

1. 우선순위페이지에서 뒤로가기 할 때 (header에 onClick reset함수 전달)
2. “시간선택 페이지에서 다음으로 버튼 누를 때”

그래서 저희는 일반적인 렌더링 흐름에 맞게 2번을 선택할 수 있을 것 같아요!
시간선택 페이지에서는 우선순위 데이터가 사용되지 않아서 꼭 뒤로가기할 때 초기화할 필요없고 다시 한번 우선순위 페이지로 들어올 때 state를 초기화해주면 될 것 같다는 생각을 했습니다.
해서 시간선택 페이지에서 TimeSlotCta의 “다음” 버튼을 누를 때 reset로직을 추가했어요

아래코드 참고해주시고 의견주시면 감사하겠습니다! 또는 제가 놓치는 부분 있으면 편히 말씀해주세요!

function TimeSlotCta() {
  const { setScheduleStep } = useScheduleStepContext();
  const { selectedSlots, setSelectedSlots } = useSelectContext();
  const isValidSelection = Object.keys(selectedSlots).length !== 0;

  const resetPriorities = (selectedSlots: SelectedSlotType) => {
    const updatedSelectedSlots: SelectedSlotType = {};
    for (const key in selectedSlots) {
      updatedSelectedSlots[key] = {
        ...selectedSlots[key],
        priority: 0,
      };
    }
    setSelectedSlots(updatedSelectedSlots);
  };
  return (
    <BtnDim>
      <Button
        typeState={isValidSelection ? 'primaryActive' : 'secondaryDisabled'}
        onClick={() => {
          setScheduleStep('selectPriority');
          resetPriorities(selectedSlots);
        }}
      >
        <Text font={'button2'}>다음</Text>
      </Button>
    </BtnDim>
  );
}

[simeunseo]

왓!! 뭔가 방법이 있을 것 같다고 생각했는데, 다음 버튼에 로직을 심는 접근 아주 좋은 것 같습니다!!! 너무 좋은 솔루션이네요 창의력 👍
SelectScheduleTable.tsx에서 기존에 넣어놨던 useEffect 로직이 아직 남아있는거같은데 이거만 좀 제거해주세요!