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

[1.2.0/ AN-UI, AN-FEAT] 배틀 날씨 효과 UI 변경 #487

Merged
merged 11 commits into from
Dec 27, 2024

Conversation

JoYehyun99
Copy link
Contributor

작업 영상

Screen_recording_20241215_165047.mp4

작업한 내용

  • 기존에는 날씨를 선택하면 아래에 무조건 설명이 나왔는데, 날씨 아이콘을 클릭하면 팝업형식으로 뜨게 하도록 변경했습니다. 🥸

PR 포인트

🚀Next Feature

  • 뷰모델 테스트
  • 최근 선택 저장 (더 좋은 방법을 고민중 ...)

@github-actions github-actions bot added AN_FEAT ✨ 안드 새로운 기능 AN_UI 🎨 안드 UI 작업 v1.2.0 🏷️ labels Dec 15, 2024
Copy link
Contributor

@sh1mj1 sh1mj1 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~

@@ -78,6 +78,9 @@ class BattleViewModel(
battleResult.map { it.isSuccess() }
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), false)

private val _showWeatherEffect = MutableStateFlow(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

weatherEffectIsShown 이 더 나을까요?
이것도 나쁘지 않은데,
이것을 사용하는 클라이언트 코드에서 볼 때 빌더 메서드가 아닌 조정자 메서드 같아 보였어요.

Copy link
Contributor

Choose a reason for hiding this comment

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

저도 동감합니다~ 근데 isShownWeatherEffect 이 어떠심? ㅋㅋㅋ

@@ -29,6 +29,8 @@ enum class WeatherIcon(
TURBULENCE(R.drawable.icon_air),
}

fun WeatherUiModel.hasWeatherEffect(): Boolean = effect.isNotBlank() && effect.isNotEmpty()
Copy link
Contributor

Choose a reason for hiding this comment

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

image

isNotBlank() 만 해도 isNotEmpty 케이스를 커버한다고 알고 있습니다.

Copy link
Contributor

@murjune murjune left a comment

Choose a reason for hiding this comment

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

UI 가 확실히 이뻐졌네요 👏

@@ -78,6 +78,9 @@ class BattleViewModel(
battleResult.map { it.isSuccess() }
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), false)

private val _showWeatherEffect = MutableStateFlow(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

저도 동감합니다~ 근데 isShownWeatherEffect 이 어떠심? ㅋㅋㅋ

Copy link
Contributor

@kkosang kkosang left a comment

Choose a reason for hiding this comment

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

UI가 훨씬 깔꼼해졌네요 고생하셨습니다~~

@@ -75,19 +75,39 @@ class BattleActivity : ToolbarActivity<ActivityBattleBinding>(R.layout.activity_
}

private fun initObserver() {
observeWeathers()
Copy link
Contributor

Choose a reason for hiding this comment

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

따로 분리하셨군요 굿 👍👍

@JoYehyun99 JoYehyun99 merged commit 52ac854 into an/develop Dec 27, 2024
2 checks passed
@JoYehyun99 JoYehyun99 deleted the an/feat/weather_effect branch December 27, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AN_FEAT ✨ 안드 새로운 기능 AN_UI 🎨 안드 UI 작업 v1.2.0 🏷️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants