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

#138 [fix] 릴리즈 전 QA 이슈 수정 #139

Merged
merged 12 commits into from
Feb 2, 2024

Conversation

pump9918
Copy link
Collaborator

@pump9918 pump9918 commented Jan 27, 2024

📑 Work Description

  • 뷰페이저 좌우 삭제
  • 카드 전환 시 이전 스택 뷰 보이는 문제 해결
  • 카드 로딩 전 기본이미지 placeholder 적용 및 로딩창 삭제

🛠️ Issue

📷 Screenshot

Screen_Recording_20240127_232246_Softie.mp4

💬 To Reviewers

@pump9918 pump9918 added UI ui 관련 작업 호연🐻‍❄️ 호연이가 작업함! fix Fix(디자인 등) labels Jan 27, 2024
@pump9918 pump9918 self-assigned this Jan 27, 2024
@stellar-halo stellar-halo changed the title [fix] 릴리즈 전 QA 이슈 수정 #138 [fix] 릴리즈 전 QA 이슈 수정 Jan 28, 2024
Copy link
Contributor

@stellar-halo stellar-halo left a comment

Choose a reason for hiding this comment

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

이미지 로딩이 느려서 고생이 많읍니다.. 저도 새로운 거 배워가요!
겹치는 코드가 많아 보이는데, 정말로 다 필요한 코드인지 확인해보시는 것도 좋을 것 같아보입니다.

Comment on lines +29 to +32
ivHappyRoutineAddCard.load(data.contentImageUrl) {
placeholder(R.drawable.ic_happy_card_base)
error(R.drawable.ic_happy_card_base)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

acitivity에 이미 있는 코드인데 여기에도 적용해주어야 하나요?

Copy link
Collaborator Author

@pump9918 pump9918 Jan 28, 2024

Choose a reason for hiding this comment

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

액티비티에는 star icon에 대한 placeholder이고, cardAdapter는 각 카드에 대한 placeholder입니다! 이 외에 delete Activity와 myRoutineFragment에도 placeholder가 되어 있을 텐데, 사용자가 카드가 다 로드 되기 전 추가하기 버튼을 눌렀을 때 임시 카드를 설정해놓기 위해서 넣어둔 겁니다!

@@ -35,6 +36,11 @@ class HappyAddListActivity :
}
}

override fun onResume() {
super.onResume()
binding.clHappyAddList.visibility = View.VISIBLE
Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 이 부분 처리해주는 이유가 뭘까요? 하나의 activity안에 있는 단 하나의 layout이라 굳이 처리해줄 필요가 없어 보입니다.

Copy link
Collaborator Author

@pump9918 pump9918 Jan 28, 2024

Choose a reason for hiding this comment

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

Screen_Recording_20240126_022836_Softie.mp4

setResult로 detailAcitivity -> AddListActivity -> MyRoutineActivity로 되돌아가는 구조인데, 이 과정에서 스택에 쌓여 있던 뷰들이 순간적으로 보여져 UX적으로 좋지 않다고 판단했습니다. onResume에 화면을 invisible 처리해두면 detailAcitivity에서 달성중 단계로 갈때 스택에 쌓인 뷰가 보이지 않아 매끄럽게 화면이 보입니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

스택에 쌓여있는 뷰들을 정리해주는 Flag가 있습니다!
공부해보시고 적절한 Flag를 쓰면 해당 문제를 해결할 수 있어 보입니다!
https://vanillacreamdonut.tistory.com/156

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

감사합니다! 자료보고 정리해보도록 하겠습니다 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

HappyAddListActivity에서는 별다른 조치 없이 DetailActivity로 넘어갈 때 finish()만 해주면 스택에 남지 않네요! 해결했습니다ㅎㅎ

@@ -89,6 +95,7 @@ class HappyAddListActivity :
putExtra(ID, id)
putExtra(ICON_IMAGE_URL, iconImageUrl)
}
binding.clHappyAddList.visibility = View.INVISIBLE
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
Collaborator Author

Choose a reason for hiding this comment

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

위와 동일한 이유로 순간적으로 보이는 뷰를 보이지 않게 처리해두었습니다. 이 방법이 바람직하지 않다면 다른 방식으로 찾아보겠습니다!

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
Member

@minemi00 minemi00 left a comment

Choose a reason for hiding this comment

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

수고하셨습니당! 근데 카드는 괜찮은데 아이콘은 기본이미지로 박아놓으니까 제껀 로딩이 더 느려지는 것 같은데 저만 그런가요..

@@ -35,7 +35,10 @@ class HappyDeleteFragment :
happyProgress?.let {
with(binding) {
tvHappyDeleteSubtitle.text = happyProgress.title
ivHappyDeleteCardFront.load(happyProgress.contentImageUrl)
ivHappyDeleteCardFront.load(happyProgress.contentImageUrl) {
placeholder(R.drawable.ic_bear_base)
Copy link
Member

Choose a reason for hiding this comment

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

쇽..샥

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

placeholder는 drawble 파일을 바로 적용하는거라 로딩이 따로 느려질 이유는 없을 것 같은데 흠... 저도 한번 확인해보겠습니다.

Copy link
Member

@emjayMJkim emjayMJkim left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!! (담번에는 좀 더 빨리 달겠습니다 ㅎㅎ

…a-issue

# Conflicts:
#	app/src/main/java/com/sopetit/softie/util/binding/BindingAdapter.kt
…a-issue

# Conflicts:
#	app/src/main/java/com/sopetit/softie/util/binding/BindingAdapter.kt
Copy link
Contributor

@stellar-halo stellar-halo left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@pump9918 pump9918 merged commit 7f4a8e9 into develop Feb 2, 2024
1 check passed
@pump9918 pump9918 deleted the feature/#138-fix-qa-issue branch February 2, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fix(디자인 등) UI ui 관련 작업 호연🐻‍❄️ 호연이가 작업함!
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[fix] 릴리즈 전 QA 이슈 수정
4 participants