Skip to content
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

Refactor/#81: UI 및 디자인 시스템 관련 오류 및 필터링 초기화 버튼 수정 #82

Merged
merged 13 commits into from
Mar 31, 2024

Conversation

semnil5202
Copy link
Member

@semnil5202 semnil5202 commented Mar 23, 2024

📄 Summary

close #81

어느새 81번까지 왔네요 30번대가 엊그제 같은데 ㅋㅋㅋㅋ

이슈에 적어둔 자잘한 UI 오류 + 필터링 초기화 및 닫기 기능 분리 적용 했습니다.

디자인 시스템 오류 수정하느라 ConceptBe/conceptbe-design-system#28 다음과 같은 변경사항이 있었으니 같이 확인해주시면 좋을 것 같습니다. 0.4.12 버전은 배포되어 있는 상태입니다. (PR은 머지 전)

FilterBottomSheet 초기화를 위해서 unmount 방식으로 수행한 부분 확인했습니다. 그렇게 할 수 밖에 없었더군요. 하지만 해당 방식이 FilterBottomSheet Pop-Up 애니메이션을 생략하는 문제가 있어서 디자인 시스템에서 useCheckbox, useRadio 훅에서 onReset00 함수를 반환하도록 했습니다. (useDropdown은 기존에 onResetDropdown이란 훅을 내포하고 있습니다.)

onResetCheckbox(checkboxkey: string);
onResetRadio({ radiokey, resetId }: { radiokey: string, resetId?: number });
onResetDropdown(dropdownKey: string);

위와 같이 사용하시면 되겠습니다. 해당하는 키 값을 선택 이전, 즉 초기화 시켜줍니다. onResetRadio 같은 경우엔 초기화 시 기본으로 선택되어야하는 옵션이 있을 수 있으므로 resetId를 옵셔널로 받을 수 있겠금 해뒀습니다. resetId를 넘기지 않으면 라디오 박스가 모두 해제됩니다.

FilterBottomSheet 초기화 로직은 unmount 방식을 제거하고 직접 초기화 하는 부분에 주석을 달아 두어 수정했습니다.

🙋🏻 More

FilterBottomSheet에서 useDropdown이 여러개 사용된 부분을 하나의 useDropdown으로 적용하였습니다.

디자인 시스템 훅의 컨셉이 하나의 form에서 하나의 훅으로 여러 개의 checkbox, input, dropdown을 관리하고자 한 것입니다. FilterBottomSheet에서 각 dropdown 컴포넌트 마다 useDropdown을 작성하신 것 같던데, 이 부분은 다음과 같이 수정할 수 있습니다.

// 하나의 훅에서 세 개의 dropdown state 관리
const {} = useDropdown({
  dropdown1: '',
  dropdown2: '',
  dropdown3: '',
});

추후 강결합 문제 때 제거될 수도 있는 훅이지만 나중에 코드 리뷰 시에 도움이 되시라고 남겨둡니다. 😇

@semnil5202 semnil5202 requested a review from yogjin March 23, 2024 15:01
@semnil5202 semnil5202 added the fix 버그 픽스 label Mar 23, 2024
@semnil5202 semnil5202 self-assigned this Mar 23, 2024
Copy link
Collaborator

@yogjin yogjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다, 궁금한점이랑 눈에 보이는 것 조금 리뷰해봤습니다

  1. 피드 필터링 바텀시트 초기화 버튼은 추가 예정인가요?
  2. 이슈에 프로필 페이지 헤더 영역 고정 문제 -> 미고정으로 변경 (재확인 결과 고정 방식이 맞음) 이게 무슨 뜻인가요?
  3. 내 프로필-아이디어도 width 수정이 필요해보입니다
image

@@ -31,47 +31,50 @@ const ErrorFallback = ({ title, children, isInApiErrorBoundary, resetErrorBounda
return (
<Flex
maxWidth={420}
height="100dvh"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100dvh 로 설정했을때 약간 왔다갔다 하는 문제가 있다는 것 같던데, 괜찮았나요?
모바일로 확인해보세요: https://mytory.net/wp-content/codes/vh-test/dist/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하 네네 100dvh로 하다보면 주소창 크기에 따라 들썩이는 부분이 발생하는데, 에러 페이지는 대부분의 기기에서 하단에 여백이 남아 에러페이지 자체가 들썩거리지는 않습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 에러 페이지의 높이가 고정이라 밑 부분이 하얗게 채워지는걸까요?
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

버튼 아래의 하얀 여백 말씀하시는 거져? 네 피그마 시안과 동일하게 두었습니다.

@semnil5202
Copy link
Member Author

피드 필터링 바텀시트 초기화 버튼은 추가 예정인가요?

엇? 초기화 버튼을 추가해두었는데 보이지 않나요?? UI가 보이지 않는건가여? 아니면 기능적으로 문제가 있는 것일까요? 제 화면은 아래처럼 보이고 기능도 정상 동작하는 걸로 확인했습니다.

image

이슈에 프로필 페이지 헤더 영역 고정 문제 -> 미고정으로 변경 (재확인 결과 고정 방식이 맞음) 이게 무슨 뜻인가요?

아 이게 단톡방에서 잠깐 얘기가 나왔던 부분인데, 아래 캡쳐 부분입니다. 아래 QA를 보면서 프로필 페이지에서 Header를 고정하는게 아닌 Static한 방식으로 두어 스크롤 시 가려지도록 해야한다 라고 이해했는데요. 재확인 결과 현재처럼 Header 를 상단에 fixed 하는게 맞다고 합니다. 그래서 저렇게 작성해두었는데 말을 너무 대충 썼었네요.

image

내 프로필-아이디어도 width 수정이 필요해보입니다.

아하 확인해보니 max-width 때는 문제가 없는데 margin으로 인해 내용물이 충분하지 않으면 width가 줄어드는 군요. 수정해서 올려두도록 하겠습니다.

@yogjin
Copy link
Collaborator

yogjin commented Mar 29, 2024

초기화버튼을 누르고 다시 필터 바텀시트를 열면 이전에 선택한게 그대로 초기값으로 표시되는 문제가 있네요
image

@semnil5202 semnil5202 merged commit 9d7d6c7 into develop Mar 31, 2024
@semnil5202 semnil5202 deleted the refactor/#81 branch April 18, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix 버그 픽스
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI 및 디자인 시스템 관련 오류 및 필터링 초기화 버튼 수정
2 participants