-
Notifications
You must be signed in to change notification settings - Fork 2
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
[시간표] 강의 수정 기능 추가 #601
[시간표] 강의 수정 기능 추가 #601
Conversation
1. 강의 수정 버튼 svg 추가 및 삭제 버튼 svg 교체 2. 강의 수정 페이지로 이동 3. 클릭한 강의 정보 불러오기 4. 정규과목 강의 수정 시 UI 수정 5. EditLectureInfo API Class 추가
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각보다 구현하기 어려운 기능이었던 것 같은데 잘 해준 것 같아요! 수고하셨습니다!!
다만, 아직 100% 완료했다고 보긴 힘들 것 같아요.
- 정규 강의 수정 시 시간 수정이 불가능한데 시간&장소 컴포넌트 삭제가 가능한 점
- 분반이 있는 강의 수정 시 시간표 내 회색 표시된(미리보기) 강의에는 분반이 없어서 자연스럽지 않음 점
- 강의 수정 완료 후 다음 행동에 대한 명세가 없어서 그대로 냅두셨지만, 수정하기 전에 정규 과목 탭에서 직접 추가 탭으로 이동한 것이기 때문에 수정 완료하면 다시 정규 과목 탭으로 이동하는 것은 어떨까요? 이 부분은 PM님, 디자이너 분과 상의하고 피그마에 명세하도록 하는 게 좋을 것 같아요!
- 강의 수정이 시간표 강의
hover
시 수정 버튼을 통해서도 가능하지만 '시간표에 추가한 과목' 리스트에서 목록 내 강의 hover시 수정 버튼을 통해서도 가능합니다! 피그마에 있는 디자인을 참고하시면 좋을 것 같아요!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
hook이 얽히고 얽히 구조가 옛날부터 있었던 것 같긴 하지만, 우선 hook들의 비슷한 기능들이 얽히면서 같은 공용훅이 재선언되고, 인자로 넘겨지고 재선언하고가 과하게 심해진 것 같아요.. 팀 내에서도 같이 이야기해보고 훅들이 일관된 기능을 가지고 공용훅 인자의 재활 같은 부분을 가능하면 제거해보면 좋을 것 같습니다!
모든 내용을 코드에 반영할 필요는 없겠지만 그래도 한 번 고민해서 수정해보시고 다시 request 부탁드립니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각보다 구현하기 어려운 기능이었던 것 같은데 잘 해준 것 같아요! 수고하셨습니다!! 다만, 아직 100% 완료했다고 보긴 힘들 것 같아요.
- 정규 강의 수정 시 시간 수정이 불가능한데 시간&장소 컴포넌트 삭제가 가능한 점
- 분반이 있는 강의 수정 시 시간표 내 회색 표시된(미리보기) 강의에는 분반이 없어서 자연스럽지 않음 점
- 강의 수정 완료 후 다음 행동에 대한 명세가 없어서 그대로 냅두셨지만, 수정하기 전에 정규 과목 탭에서 직접 추가 탭으로 이동한 것이기 때문에 수정 완료하면 다시 정규 과목 탭으로 이동하는 것은 어떨까요? 이 부분은 PM님, 디자이너 분과 상의하고 피그마에 명세하도록 하는 게 좋을 것 같아요!
- 강의 수정이 시간표 강의 hover시 수정 버튼을 통해서도 가능하지만 '시간표에 추가한 과목' 리스트에서 목록 내 강의 hover시 수정 버튼을 통해서도 가능합니다! 피그마에 있는 디자인을 참고하시면 좋을 것 같아요!
- 이 부분은 따로 삭제 불가능하도록 수정하겠습니다!
- 명세가 없어서 의견이 다를 수 있지만 우선 저의 의도를 말씀드리자면, 제가 생각하기에 정규 과목 수정 시 분반 수정이 어렵고 강의 정보를 수정함과 동시에 미리보기와 기존 강의 내용이 달라져 결국엔 부자연스럽게 보인다고 생각합니다. 이에 대해서 정규 과목 수정 시 분반을 함께 출력하여 조금 더 자연스럽게 출력할 것인지 아님 미리보기를 없앨지 등에 대한 다양한 방향이 있을 것 같았고, 명세가 나온 후에 이 부분을 수정하는 것이 나을 것 같아 이 부분은 대처하지 않았습니다. 그래도 분반 출력 부분은 한 번 더 고민해보겠습니다!
- 이 부분 또한 PM님과 디자이너 분과 명세에 대해 논의해보겠습니다!
- 이 부분에 대해서 PR 선공지를 못한 것 같습니다..! PR Post에 적어두고 디자인팀에게 확인해본 후 추가하겠습니다!
위에서 말씀하신 부분은 반영을 못하신 것 같아요! 이 부분과 timeSpaceComponent
끼리 시간이 겹치는 경우의 처리 로직만 반영하면 될 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
대면 리뷰로 대신했습니다! 수고하셨습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
What is this PR? 🔍
Changes 📝
ScreenShot 📷
[비로그인 상태에서 강의 수정 버튼 클릭 시 경고 토스트 출력]
[강의 수정 버튼]
[커스텀 강의 수정 시 포맷]
[정규 강의 수정 시 포맷]
[강의 수정 완료 될 시 완료 토스트 출력]
[강의 수정 사항 반영]
✔️ Please check if the PR fulfills these requirements
develop
branch unconditionally?main
?yarn lint