-
Notifications
You must be signed in to change notification settings - Fork 4
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
경매 상세 페이지 수정(마감기한 시간 정보 뷰 수정 및 최고 입찰자 표시) #699
Conversation
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.
고생하셨습니다!!
private fun getRemainingHeight(context: Context): Int { | ||
val screenHeight = context.resources.displayMetrics.heightPixels | ||
val statusBarHeight = convertDpToPx(24f) // 탭 레이아웃의 톱 마진과도 같은 값임. |
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.
수고했습니다~!
기종별로 모두 테스트해보시다니 👍 👍 👍
정말 대단해요!!!! 👏
@@ -41,17 +42,19 @@ class AuctionDetailActivity : | |||
// 액션바 높이를 반환하는 함수 | |||
private fun getActionBarHeight(context: Context): Int { | |||
val styledAttributes = | |||
context.theme.obtainStyledAttributes(intArrayOf(android.R.attr.actionBarSize)) | |||
context.theme.obtainStyledAttributes(intArrayOf(com.google.android.material.R.attr.actionBarSize)) |
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.
androidx.appcompat.R.attr.actionBarSize
를 사용하는건 어때요?
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.
상태바 사이즈가 포함되지 않은 스크린 높이를 구할 수 있는 api를 찾았는데 확인 부탁드려요ㅎㅎ 😉
@@ -260,13 +261,16 @@ | |||
<com.google.android.material.tabs.TabLayout | |||
android:id="@+id/tb_detail_info" | |||
android:layout_width="0dp" | |||
android:layout_height="wrap_content" | |||
android:layout_height="48dp" |
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.
@dimen/height_submit_button
으로 하는게 좋을 것 같아요!
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.
그걸 사용할까 하다가 안했던 이유는, 버튼이 아니라 탭 레이아웃에 대한 높이인데 이걸 적용하면, 나중에 리팩토링하다가 예기치 못하게 같이 바뀔 수 있을 것 같아요.
private fun getRemainingHeight(context: Context): Int { | ||
val screenHeight = context.resources.displayMetrics.heightPixels | ||
val statusBarHeight = convertDpToPx(STATUS_BAR_HEIGHT) // 탭 레이아웃의 톱 마진과도 같은 값임. | ||
val actionBarHeight = getActionBarHeight(context) | ||
return screenHeight - actionBarHeight * 2 | ||
val tabLayoutHeight = resources.getDimensionPixelOffset(R.dimen.height_submit_button) | ||
val bottomButtonHeight = resources.getDimensionPixelOffset(R.dimen.height_submit_button) | ||
return screenHeight - statusBarHeight - actionBarHeight - tabLayoutHeight - bottomButtonHeight | ||
} |
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.
private fun getRemainingHeight(context: Context): Int {
val screenHeight = convertDpToPx(resources.configuration.screenHeightDp.toFloat())
val actionBarHeight = getActionBarHeight(context)
val tabLayoutHeight = resources.getDimensionPixelOffset(R.dimen.height_submit_button)
val bottomButtonHeight = resources.getDimensionPixelOffset(R.dimen.height_submit_button)
return screenHeight - actionBarHeight - tabLayoutHeight - bottomButtonHeight
}
resources.configuration.screenHeightDp
를 사용하면 화면 orientation, cutout 사이즈를 고려하지 않아도 될 것 같아요!- screenHeightDp
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.
찾아주셔서 감사합니다! 테스트 중에 픽셀2 api33 버전에서 상태바가 같이 나오고 있는 것 같아서 확인이 필요할 것 같아요!
기존의 동적 뷰페이저 높이 설정에 문제가 많다고 생각이 들어서, CoordinatorLayout을 사용하도록 수정했습니다. 또한, 가로모드에 대한 대응을 적용한 UI도 추가했습니다. 아래 자료를 참고했습니다. CoordinatorLayout에 대해서는 이미 자료가 medium에 많아서 찾아보시면 좋을 것 같습니다 |
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.
CoordinatorLayout이랑 CollapsingToolbarLayout로 수정하느라 수고했어요! 👍 👍 👍
정말 멋져요!!! 딱 원하던 기능이에요!!! 👏
📄 작업 내용 요약
경매 상세 페이지 수정
🙋🏻 리뷰 시 주의 깊게 확인해야 하는 코드
=> 우선, 여기서 파란색 박스는 기존에 있던 로직에서 액션바의 속성을 잘못 선택해서 오차가 발생하고 있던 것입니다. 그 이유는 다음과 같습니다. 요약하자면, 액션바 속성이 두 개가 있는데 그 중 다른걸 사용하고 있어서 수정했습니다.
https://stackoverflow.com/questions/26449195/new-theme-appcompat-actionbar-height
빨간색 박스 로직에 대한 설명)
=> 여기서, getRemainHeight에서 Context.resources.displayMetrics.heightPixels 는 상태바 높이 + 액티비티 높이(이 안에 툴바도 포함)를 반환합니다. 이 메소드는 네비게이션 바의 높이를 안주기 때문에 큰 장점이 있습니다. 네비게이션 바는 모드가 기종마다 혹은 회사마다 여러 가지라서 개발자가 신경쓰기 힘든 부분인데 이를 고려대상에서 제외한 사이즈를 주기 때문에 아주 나이스합니다.
또한, 안드로이드에서 상태바의 높이는 24dp입니다.
그렇다면, 저희는 다음과 같이 빼면 정말 딱 맞는 뷰페이저의 높이를 구할 수 있습니다.
Context.resources.displayMetrics.heightPixels - 상태바 높이 - 툴바 높이 - 탭레이아웃 높이 - 상세페이지의 하단 입찰 버튼 높이
하지만, 여기서 문제가 있습니다.
갤럭시의 경우, 펀치홀이 상태바에 있는 세로모드의 경우, Context.resources.displayMetrics.heightPixels 에서 상태바 높이가 이미 빠진 상태로 나오게 된다는 결론을 내리게 되었습니다(api28, 33 버전의 픽셀2 휴대폰에서는 세로모드와 가로모드에서 모두 상태바가 포함된 높이가 나왔습니다. 또한, 갤럭시의 가로모드에서도 상태바 높이가 포함된 상태로 나오는 것을 확인했습니다. 오직, 갤럭시의 세로모드에서만 상태바 높이가 빠진 상태로 높이가 나오는 것을 실험 결과로 알게 되었습니다).
그렇다고, 로직을 아래와 같이 수정하기에는 다른 기종이나 갤럭시의 가로모드에서 대응이 안됩니다.
Context.resources.displayMetrics.heightPixels - 툴바 높이 - 탭레이아웃 높이 - 상세페이지의 하단 입찰 버튼 높이
때문에, 그냥 상태바 높이인 24dp~25dp 중 더 작은 값인 24dp로 빼주기로 했습니다. (여기서 더 작은 값을 빼는 이유는, 뷰페이저가 1dp 길어지는 것은 상관없으나, 1dp가 짧아지면 보기 안좋기 때문입니다. 모든 기종에서 고르게 보이기 위해 더 작은 24dp를 빼주기로 했습니다.
(만약 25dp를 빼면, 뷰페이저가 그만큼 짧아지게 되고, 그로인해 탭 레이아웃 위에있는 'x명 경매중' 이라는 텍스특뷰의 하단 부분이 튀어나오게 되어서 미관상 보기 안좋습니다.)
그나마 다행인 점은 저희가 탭레이아웃의 마진톱을 24dp로 해놔서, 펀치홀이 있는 갤럭시 세로모드에서도 그렇게 부자연스럽지 않습니다.
실기기 사진은 필요하면, 내일 현장에서 보여드리겠습니다.
위와 같은 결론을 제가 내린 다음에, 자료를 찾아보다가 다음과 같은 저와 같은 결론을 내린 사람의 글을 발견했습니다. 우선, 저희 minSdk가 28 P인 점을 고려하고 확인하시면 될 것 같습니다.
https://black-jin0427.tistory.com/230
추가적으로, 경매 상세 페이지에서 바텀 하단 버튼과 뷰페이저 스크롤 영역이 겹치지 않도록 했습니다. marginBottom 값을 주어서 해결했습니다. 이 과정에서 바텀 버튼의 높이가 커지면 안되기 때문에 글자 크기를 고정으로 18dp로 설정했습니다.
📎 Issue 번호