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

Feature/add delete like #61

Merged
merged 5 commits into from
Jan 25, 2025
Merged

Feature/add delete like #61

merged 5 commits into from
Jan 25, 2025

Conversation

joongwon0204
Copy link
Collaborator

@joongwon0204 joongwon0204 commented Jan 25, 2025

PR 타입(하나 이상의 PR 타입을 선택해주세요)

-[O] 기능 추가

-[] 기능 삭제

-[] 버그 수정

-[] 의존성, 환경 변수, 빌드 관련 코드 업데이트

반영 브랜치


Feature/add delete like -> main

변경 사항


좋아요 삭제 기능 추가 됨

추후 보완 사항

2025-01-26.01.26.10.mov

@joongwon0204 joongwon0204 requested a review from muzigae January 25, 2025 16:25
Copy link
Collaborator

@muzigae muzigae left a comment

Choose a reason for hiding this comment

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

2025-01-26.3.06.07.mov

확인했습니다. 기본적인 API 기능은 다 잘 구현해주신 것 같은데 새로고침과 dismiss 이슈가 있습니다. 영상을 보시면 같은 글에 대해 좋아요를 누를 때 실시간으로 좋아요가 업데이트 되어야 하는 상황이 떠야 할 것 같기도 한데 제가 티스토리를 살펴 보니 '이 블로그의 "다른 글"' 이라서 같은 글을 새로고침 해야 할 일 자체가 발생하지 않는 것 같아요.
두 번째로는 영상을 보시면 뒤로 갔을 때 좋아요를 누른 것이 업데이트가 되지 않아 문제가 생기는데 dismiss가 가끔 상태를 보존해서 이런 문제가 있는 것 같더라고요. (물론 해당 뷰에서만 그렇고 서버에는 좋아요가 정상적으로 올라간 상태인 것 같긴 해요.) 물론 첫 번째 이슈를 해결하면 a->a로 가는 상황은 없겠지만 a->b->a로 가는 상황에서도 a 글에 대한 dismiss 이슈가 충분히 발생할 수 있어서 이 점을 고쳐야 할 것 같습니다. 일단 올려주신 사항들은 문제가 없어 보여서 머지하시고 수정하는 브랜치를 만드셔도 되고 아니면 여기서 수정하셔도 될 것 같습니다. 일단 approve로 놓고 답변 기다리겠습니다.
그리고 PR이 두 개라서 머지 충돌 조심하시면 좋을 것 같아요.

@joongwon0204 joongwon0204 requested a review from muzigae January 25, 2025 20:24
@joongwon0204
Copy link
Collaborator Author

postViewModel에 있는 postList 들에서 자기 자신을 제거하는 기능을 추가해서 이제 자기 자신을 새로고침하는 일이 없습니다. 좋아요가 업데이트 되지 않는 이슈는 postView에서 아직 onAppear할 때 서버에서 정보를 가져오지 않아서 그렇습니다. article content를 가져오면서 정보를 새로 불러오게 바꾸면 해결될 것으로 보입니다. PostView, BlogView 모두 똑같이 ID를 받고, 그 ID로 각자 필요한 모델을 get하는 형식으로 수정할 예정입니다.

@muzigae
Copy link
Collaborator

muzigae commented Jan 25, 2025

확인했습니다. 병합하셔도 될 것 같습니다. 구현하신 다음 잘 작동하는지 다시 확인하면 되겠네요.

@joongwon0204 joongwon0204 merged commit 1c3826d into main Jan 25, 2025
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.

2 participants