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/boolti 347 웹뷰 브릿지 연결 (공연 등록) #358

Merged
merged 15 commits into from
Jan 11, 2025

Conversation

mangbaam
Copy link
Member

@mangbaam mangbaam commented Dec 25, 2024

Issue

작업 내용

2024-12-25.6.27.14.mov
2024-12-31.2.23.13.mov
image

관계도

image

@mangbaam mangbaam requested a review from HamBP December 25, 2024 09:30
@mangbaam mangbaam self-assigned this Dec 25, 2024
Copy link

github-actions bot commented Dec 30, 2024

Test Results

 9 files   9 suites   0s ⏱️
 8 tests  8 ✅ 0 💤 0 ❌
12 runs  12 ✅ 0 💤 0 ❌

Results for commit 18ad02f.

♻️ This comment has been updated with latest results.

@mangbaam mangbaam added the feat 새로운 기능 label Dec 31, 2024
@mangbaam mangbaam marked this pull request as ready for review December 31, 2024 05:28
# Conflicts:
#	presentation/src/main/java/com/nexters/boolti/presentation/screen/Main.kt
#	presentation/src/main/java/com/nexters/boolti/presentation/screen/home/HomeScreen.kt
#	presentation/src/main/java/com/nexters/boolti/presentation/screen/showregistration/ShowRegistrationNavigation.kt
Copy link
Member

@HamBP HamBP 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 +70 to +84
try {
// JSON 메시지 파싱
val dto = Json.decodeFromString(BridgeDto.serializer(), message)
bridgeManager.handleIncomingData(dto)
} catch (e: SerializationException) {
Timber.tag("webview_bridge")
.e(e, "(WEB -> APP) 유효하지 않은 JSON 포맷: $message")
} catch (e: IllegalArgumentException) {
Timber.tag("webview_bridge")
.e(e, "(WEB -> APP) BridgeDto 타입으로 파싱 실패: $message")
} catch (e: Exception) {
Timber.tag("webview_bridge")
.e(e, "(WEB -> APP) 알 수 없는 에러")
e.printStackTrace() // 에러 처리
}
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
Member Author

Choose a reason for hiding this comment

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

crashlytics 를 말한 거라면 이미 catch하고 있어서 그거보다는 추후에 로그 플랫폼 정해지면 로그 작업 풀로 들어가야 할 거 같긴 해

Comment on lines +122 to +126
override fun onConsoleMessage(message: ConsoleMessage?): Boolean {
Timber.tag("webview_console Message bridge")
.d("${message?.message()} -- From line ${message?.lineNumber()} of ${message?.sourceId()}")
return true
}
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
Member Author

Choose a reason for hiding this comment

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

예압~~ 너무 좋더라고

*
* 예를 들어 화면 이동의 경우 웹으로 전달할 데이터가 없으니 [BridgeDto.timestamp]를 제외한 나머지 정보는 에코 방식으로 그대로 콜백한다.
*
* 토큰 요청과 같이 웹으로 전달할 데이터가 있는 경우 [BridgeDto.data]에 웹과 맞춘 형태로 데이터를 포함시키고, 나머지 [BridgeDto.id]와 [BridgeDto.command]는 그대로 콜백한다.
Copy link
Member

Choose a reason for hiding this comment

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

[질문] 그러면 토큰의 경우 웹에서 직접 refresh하지 않고 앱에 요청하는 방식인 건가?

Copy link
Member Author

Choose a reason for hiding this comment

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

맞아 토큰 관련된 제어는 모두 앱에서 처리해

Comment on lines +6 to +11
enum class CommandType {
NAVIGATE_TO_SHOW_DETAIL,
NAVIGATE_BACK,
REQUEST_TOKEN,
SHOW_TOAST,
UNKNOWN;
Copy link
Member

Choose a reason for hiding this comment

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

[질문] 액션이 이 친구들 말하는 건가?

새로운 액션이 정의되는 경우 [BridgeManager.handleIncomingData] 도 함께 수정되어야 한다.

Copy link
Member Author

Choose a reason for hiding this comment

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

맞습니다!

Comment on lines 48 to 61
CommandType.NAVIGATE_TO_SHOW_DETAIL -> {
data.data?.jsonObject?.get("showId")?.toString()?.let { showId ->
scope.launch {
withContext(Dispatchers.Main) {
Timber.tag("bridge").d("공연 상세 화면으로 이동 $showId")
callbackHandler.navigate(
route = ShowRoute.ShowRoot(showId),
navigateOption = NavigateOption.CLOSE_AND_OPEN,
)
}
}
} ?: Timber.tag("bridge").d("공연 상세 화면으로 이동 실패: showId 없음")
callbackToWeb(data)
}
Copy link
Member

Choose a reason for hiding this comment

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

[질문] navigate에 coroutineScrope를 쓴 이유가 뭐야?

Copy link
Member Author

Choose a reason for hiding this comment

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

Main 스레드에서 돌리기 위한 목적이었는데, 지금 생각해보니 runOnUi 써도 되겠다

callbackToWeb(data)
}

else -> callbackToWeb(data)
Copy link
Member

Choose a reason for hiding this comment

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

새로운 액션이 정의되는 경우 [BridgeManager.handleIncomingData] 도 함께 수정되어야 한다.

else 대신 각각의 command를 명시하면 command type 추가됐을 때 컴파일 에러로 체크할 수 있을 것 같아

Copy link
Member Author

Choose a reason for hiding this comment

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

그것도 맞는 말인데, 지금 fe랑 협의된 내용은 항상 callback 을 해준다는 거였어. 그래야 FE 에서 비동기 통신을 마칠 수 있대
둘 다 충족할 수 있는 좋은 방법 생각나면 추천해줘

Copy link
Member Author

Choose a reason for hiding this comment

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

일단 else 문 없애고 callbackToWeb(data) 호출하도록 변경함

@mangbaam mangbaam merged commit 21ff010 into develop Jan 11, 2025
1 check passed
@mangbaam mangbaam deleted the feature/Boolti-347 branch January 11, 2025 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat 새로운 기능
Projects
None yet
Development

Successfully merging this pull request may close these issues.

웹뷰 브릿지
2 participants