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

[fix/#58]: 과목 편집 문제 해결 #89

Merged
merged 10 commits into from
Aug 26, 2024
Merged

Conversation

doyi0107
Copy link
Contributor

@doyi0107 doyi0107 commented Aug 22, 2024

📋 연관된 이슈 번호

🛠 구현 사항

  • 과목 추가 시 중복 과목 예외 처리
  • 변경 사항 없음 예외 처리

📚 변경 사항

  • 과목 편집에서 똑같은 명칭으로 과목 추가하고 삭제하면 업데이트 실패가 뜨는 거 해결
  • deletedSubjects, addedSubjects,selectedSubjects 모두 객체로 관리하고 deletedSubjects, addedSubjects 서버로 보낼 때만 각각 id값과 네임명으로 보내는 방식으로 바꾸면서 해결,,,
  • 과목명 엄청 길게 쓰면 삭제 버튼이 오른쪽으로 밀려서 사라짐: 재연언니가 알려준 방식으로 고침

주말 동안 수정한 사항

  • 인스턴스api에 있는 중복 코드 삭제 , error code 수정
  • 알림만 : 과목 삭제 id, 과목 추가 name ->빈 배열시 알림x , 과목 삭제,추가 다 name명으로
  • 이 과목은 이미 존재합니다 -> (특정 과목 이름)은 이미 존재합니다.
  • 랭킹 탭이랑, 풀 랭킹만 이용해서 아래, 위 여백만 늘려줬는데 괜찮을까요?

💬 To Reviewers

  • subjectStore는 점점 더 코드가 길어져 내일 + 주말 동안 고치겠습니다 -> 이거이거,, 공통으로 관리되고 있는 것들을 따로 빼주고 추가, 삭제, 리셋,저장 기능 나눠서, 따로 빼준 공통 store는 다른 store에서 불러서 쓰려다가 에러를 만남,, 문법 오류는 안 나는데 불러와서 쓰는 과정에서 업데이트가 안되는 것들이 많이 생김. issue파서 거기다가 수정하겠습니다. 정 안되면 강사님한테 물어보려고요!

Copy link
Contributor

@riverkite0708 riverkite0708 left a comment

Choose a reason for hiding this comment

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

수고많았어요~! 👏👏👏

.subject_item_wrapper {
position: relative;
display: inline-block;
max-width: 99%;
Copy link
Contributor

Choose a reason for hiding this comment

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

왜 최대 넓이를 99%만 줬는지 궁금합니다~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

100%로 하면 (-)삭제 버튼이 살짝 짤리던데 width값을 줄이는 거 말고 다른 방법이 있을까요?!

Copy link
Contributor

Choose a reason for hiding this comment

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

overflow-x 때문인데 전에도 overflow-y 때문에 감싸고 있는 박스에 padding-top:5px을 줬었죠~ 마찬가지로 하면 됩니다^_^
그런데 padding-right를 주면 좌우여백이 조금 달라지니 margin-right:-5px로 당겨주면 더더더 좋겠습니다~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이거 다시 해봤을 때 max-width:100%로 해도 큰 문제가 없는 것 같아서 100%로 수정했습니다,,!

const isDuplicate = subjects.some((subject) => subject.name === newSubjectName.trim());
if (isDuplicate) {
alert('이 과목은 이미 존재합니다.');
return;
Copy link
Contributor

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.

굳굳

if (requestBody.addedSubjects.length === 0 && requestBody.deletedSubjects.length === 0) {
return { success: true }; // 변경 사항이 없으므로 성공으로 처리
}

return { success: true };
} catch (error) {
console.error('subjectFormApi 에러:', error);
Copy link
Member

@Kong-E Kong-E Aug 23, 2024

Choose a reason for hiding this comment

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

error code에 따라 메시지를 다르게 전달했으면 좋겠어요~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영하겠습니다!!

@doyi0107 doyi0107 merged commit 6196f85 into dev Aug 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants