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

feat : 약 페이지 개발 #127

Merged
merged 40 commits into from
Sep 29, 2024
Merged

Conversation

KimDongGyun23
Copy link
Contributor

변경 사항

  • 현재 백엔드에서 약 정보 삭제하는 부분에서 이슈가 있습니다. 수정은 되었으나 아직 서버가 재배포되지 않아 확인은 불가할 것입니다.

  • 공통 컴포넌트가 아닌 약 페이지에 종속된 컴포넌트들을 해당 페이지의 컴포넌트로 이동시켰습니다. (Stepper, Toggle)

  • 공통 컴포넌트로 이동할 수 있는 부분들은 공통으로 이동시켜 사용하였습니다. (alarmBottomSheet, deleteModal)


수정 필요

  • api 요청 시, 로딩과 에러 상태에 대한 부분
  • Hydration 기능

close #126

InputGroup의 Stepper를 medicine의 feature로 이동
추가, 수정을 Link태그로 감싸며 서버 컴포넌트로 이전
clinicInfo 모달 삭제 및 공통컴포넌트로 가져오기
MedicineDetail에서 공통컴포넌트 가져오기 및 api 연결
stepper 1~5까지로 제한
선택 날짜 전체 제거 후 입력 시 오류 수정
선택 날짜 없을 시, 오류 메세지 추가
@KimDongGyun23 KimDongGyun23 self-assigned this Sep 24, 2024
src/constants/medicine.ts Outdated Show resolved Hide resolved
src/features/medicine/query/queryKeys.ts Show resolved Hide resolved
src/features/medicine/query/queryKeys.ts Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

DeleteModal의 형식(제목, 버튼 2개)을 공통 컴포넌트로 만들어서 제목, 멘트, 버튼명 props를 받아서 따로 사용하는거 어떻게 생각하시나요??? 가족 설정 페이지에도 해당 모달과 동일한 형식의 모달을 사용하고 있는데 공통적으로 사용하면 좋을 것 같아서요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 저도 공통 Modal 컴포넌트를 ( 버튼 모달, 초대 코드 모달 ) 이 두가지를 생각했었는데,
다른 방식으로 구현하셨어서 다른 분들 작업 진행 과정에 영향가지 않도록 제 것만 따로 만들어 둔거였습니다.

저는 요청하신 방안에 동의합니다.
태윤님께서 모달 컴포넌트 수정하실 때, 이 부분도 반영해주시면 좋을 것 같네요.

Copy link
Contributor

Choose a reason for hiding this comment

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

@G0MTENG 좋습니다~!

@kimsuyeon0916 kimsuyeon0916 self-requested a review September 25, 2024 13:59
Copy link
Contributor

@kimsuyeon0916 kimsuyeon0916 left a comment

Choose a reason for hiding this comment

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

동균님 고생하셨습니다! 수정 부분에 이슈가 있어서 확인해보셔야 할 것 같고 삭제 부분은 아직 테스트를 못했습니다~! 서버 정상화 되는데로 말씀해주시면 확인해보겠습니다~!

src/features/medicine/ui/MedicineList.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

image

화살표 부분(>) 좌측의 글씨가 살짝씩 잘리는데 확인해보셔야 할 것 같습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

image

여기서 알람 주기를 선택했을 때, 유저가 해당 알람을 클릭해도 변화가 없어서 설정된건지 완료 버튼을 누르기 전까지는 해당 페이지에서 확인하기 어려울 것 같은데 선택하고 나서 바텀시트가 닫히는 게 하는거 어떻게 생각하세요~?

Copy link
Contributor

Choose a reason for hiding this comment

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

image

복용횟수 2회로 추가하고 나서 수정 페이지에 들어가면 복용 횟수는 1로 되있어서 해당 부분 확인해보셔야 할 것 같습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

복용 기간 부분에서 오늘보다 이전 날짜를 선택해도 되는데, 오늘보다 이전 날짜를 선택하지 못하도록 해야 할 것 같은데 어떻게 생각하세요??

또한 특정 날짜 특정 년도를 선택해도 2024년도로 들어가는 버그가 있습니다..! 확인해보셔야 할 것 같아요

그리고 특정 날짜를 선택하고 나서(하이라이트가 되있는 상태에서) 캘린더의 월을 변경했을 때(다음/이전 버튼 클릭 후)에도 아까 선택한 일자가 다른 월에도 하이라이트 되는 데 해당 부분은 의도하신 건가요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

우선 이전 날짜와 특정 년도에 대해서는 저도 확인을 했었지만, 기획이나 디자인 및 다른 파트에서 누락된 부분이 많아서 끝난 후 리팩토링 과정에서 말씀드릴려고 했었습니다. ( 조금 조금 수정하기에는 양이 많다고 생각해서 한번에 정리하고 다같이 몰아치려 했었습니다. )

  1. 현재 서비스에서 시작 날짜를 선택할 수 있는 부분이 없습니다. 따라서 서버에는 항상 오늘 날짜를 시작 날짜로 넘겨줍니다. 이 부분에 대해서 고민되던 부분은 다음과 같습니다.
  • 만약, 이전 날짜부터 먹고 있던 경우는 기록을 못하는가?
  • 그렇게 된다면, 말씀하셨던 것처럼 이전 날짜를 선택하지 못하게 해야하는가?

  1. 입력 폼에서의 복용 기간에 년도를 표시해야 할 것 같습니다.
  • 만약, 내년을 선택했을 경우 UI를 보고 수정할 경우 헷갈리지 않을까란 생각이 들었습니다.
  • 그렇다면 UI에 맞게 월/일만 표시할 경우에는 내년을 선택할 수 없게 해야하는가?

  1. 복용약의 타입을 '회분'으로 기입 시, 문제 사항들이 조금 있었습니다. '회분'을 입력을 받는 것이 과연 맞는 것일까?
  • 3회분으로 선택했을 경우, 약 세부 정보에서 각 시간 별로 '3회분'으로 표시됩니다. 사용자 입장에서는 시간별로 3회분 치를 먹으라는 것인가?
  • 개인적으로 '회분' 이라는 개념과 복용 횟수의 개념과 겹쳐서 헷갈리겠다는 생각이 들었습니다.

  1. 약 페이지와는 별개이기는 하지만, 약 복용 여부를 클릭하는 경우를 보고 생각난 부분입니다.
  • 진료 페이지에서, 방문 여부를 클릭하는 부분이 api가 빠져있는 것 같습니다.
  • 만약, 그런 기능을 생각하지 않았던 것이라면 체크 박스 UI가 삭제되어야 한다고 생각합니다.



기타 등등 다양한 기획 관련 이슈들이 많았습니다만, 해당 부분에서 고민하던 과정에서 확정짓지 못하고 애매하게 구현된 부분들이 있었던 것 같습니다. 이와 관련해서는 다같이 회의를 통해 명확하게 확정을 짓고 다시 수정하는 것이 맞다고 생각하여 일단은 구현해두긴 했습니다.

단순히 저희만 수정할 수 없는 사항들도 많아서 전체 회의를 통해서 결정했으면 좋겠습니다.
일단 그 전까지는 해당 부분으로 구현해놓겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

상단의 알람 선택과 수정 페이지 부분의 복용 횟수 부분은 수정해두겠습니다!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

화살표 부분(>) 좌측의 글씨가 살짝씩 잘리는데 확인해보셔야 할 것 같습니다!

이 부분에 대해서는 실제 처리도 되어있고, 잘 돌아가는 것이 확인이 되었습니다.
혹시, 이전 커밋에 대한 실행인가요?
아니라면 해상도와 어떤 환경에서 실행된건지 알려주실 수 있으실까요? 확인해보겠습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

단순히 저희만 수정할 수 없는 사항들도 많아서 전체 회의를 통해서 결정했으면 좋겠습니다. 일단 그 전까지는 해당 부분으로 구현해놓겠습니다!

넵! 알겠습니다~! 이슈화만 해놓으면 좋을 것 같네용

이 부분에 대해서는 실제 처리도 되어있고, 잘 돌아가는 것이 확인이 되었습니다. 혹시, 이전 커밋에 대한 실행인가요?
아니라면 해상도와 어떤 환경에서 실행된건지 알려주실 수 있으실까요? 확인해보겠습니다.

따로 해상도를 설정하지 않았고 저희 기본 레이아웃에서 확인하였고 최신 커밋에서 실행하였습니다!

src/features/medicine/ui/MedicineEditClientPage.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@G0MTENG G0MTENG left a comment

Choose a reason for hiding this comment

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

수고 많으셨습니다 :D

@KimDongGyun23 KimDongGyun23 merged commit 1561f41 into develop Sep 29, 2024
1 check passed
Copy link

@KimDongGyun23 KimDongGyun23 deleted the feature/#126_medicine_page branch September 29, 2024 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat : 약 페이지 개발
3 participants