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

[QA] 디테일 스크린 해상도 대응 #239

Merged
merged 5 commits into from
Sep 30, 2024
Merged

Conversation

hj1115hj
Copy link
Collaborator

@hj1115hj hj1115hj commented Sep 19, 2024

Issue

작업 내역 (Required)

  • detail 스크린의 height 사이즈 별로 small, medium, large로 구분
  • small일때 texts 영역 image와 겹쳐서 보이도록 구조 벼경
  • small일때 image에 그라데이션 추가
  • small일때 button, text 컬러 변경
  • small, medium, large일때 image 사이즈 변경

Review Point (Required)

  • 정의해준 기준에서 W가 360이하일때가 small로 판단하는데 디테일스크린에서 모든 component가 배치가 세로로 되어있고 정렬 기준이 가로센터로 되어있어서 가로사이즈 변경에 의해 잘리는 부분이 없어서 W 사이즈는 small로 구분하는데 사용안했습니다!

image

Screenshot

test용으로 W,H dp사이즈 출력했고 commit에서는 지웠어염
https://github.com/user-attachments/assets/672f9221-84de-4003-b031-513a9a1b35fd

@hj1115hj hj1115hj force-pushed the qa/detail_screen_size branch from 20c8919 to a3b8815 Compare September 23, 2024 14:54
Copy link
Collaborator

@ze-zeh ze-zeh left a comment

Choose a reason for hiding this comment

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

굳굳 고생했어요!!! 빨리 작업해야겠다 나도!

@@ -26,6 +26,21 @@ data class DetailUiState(
isError = false,
isLoading = false,
)

val PREVIEW_STATE = DetailUiState(
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 이렇게 preview state 뺴놓는거 좋다~ 참고할게요!!

@@ -188,7 +201,7 @@ internal fun DetailHashTags(
shape = FarmemeRadius.Radius4.shape,
),
text = name.truncateDisplayedString(16),
color = FarmemeTheme.textColor.primary,
color = if (currentDetailScreenSize == DetailScreenSize.SMALL) FarmemeTheme.textColor.inverse else FarmemeTheme.textColor.primary,
Copy link
Collaborator

Choose a reason for hiding this comment

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

개인적으로는 textColor 변수로 빼는게 깔끔해 보이더라구요~

Copy link
Collaborator Author

@hj1115hj hj1115hj Sep 26, 2024

Choose a reason for hiding this comment

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

오 깔끔하네용! 리뷰감사해용! 반영완룡!

Comment on lines +7 to +9
SMALL(360.dp, 640.dp),
MEDIUM(360.dp, 800.dp),
LARGE(430.dp, 932.dp);
Copy link
Collaborator

Choose a reason for hiding this comment

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

요 size들은 어떤 기준으로 정해진거야?
뭔가 관련있다면 요고를 참고해봐도 좋을 듯 하기도 하고?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ui에 적힌 기준이였엉! 오 !! 안드전용으로 다시 바꿔달라구 해야겠다~ 감사합니당 ><

Copy link
Collaborator

@EvergreenTree97 EvergreenTree97 left a comment

Choose a reason for hiding this comment

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

구웄~

LARGE(430.dp, 932.dp);

companion object {
fun from(width: Dp, height: Dp): DetailScreenSize {
Copy link
Collaborator

Choose a reason for hiding this comment

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

enum class 및 from 함수 정의 좋은데? 직관적이네유~

@@ -323,11 +339,12 @@ fun DetailFunnyButton(
fun PreviewDetailContent() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Preview Private으로 숨기면 좋을듯~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

앗 까먹구있었당 고마옹!! 반영 완룡!

@hj1115hj hj1115hj merged commit 3030859 into develop Sep 30, 2024
@hj1115hj hj1115hj deleted the qa/detail_screen_size branch September 30, 2024 14:16
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.

Detail 해상도 대응
4 participants