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

fix: launchApp 내 IOS <-> android 작동 코드 서로 변경 #121

Merged
merged 5 commits into from
Jan 5, 2024

Conversation

syonkr
Copy link
Contributor

@syonkr syonkr commented Jan 4, 2024

android canLaunch 관련 이슈로 인해 launchApp 코드를 변경하였습니다.

@syonkr syonkr changed the title IOS <-> android launchApp 코드 변경 fix: IOS <-> android launchApp 내 코드 변경 Jan 4, 2024
@syonkr syonkr changed the title fix: IOS <-> android launchApp 내 코드 변경 fix: launchApp 내 IOS <-> android 작동 코드 서로 변경 Jan 4, 2024
@anymate98
Copy link
Contributor

안녕하세요. 우선 코드 기여 감사합니다.
이번에 앱 실행 방식을 변경하면서 고려했던 것은 url_launcher을 사용했을 경우 ios에서는 앱이 없어서 실행되지 않아도 exception이 발생하지 않아 정상적으로 리턴값을 받을 수 있었던 반면 안드로이드의 경우 앱이 없어서 앱을 여는데 실패하면 exception이 발생한다는 점입니다. 이것이 안드로이드에서 canLaunch를 추가한 이유고 만약 이것을 고려해 canLaunch 체크를 생략하려면 이전처럼 try catch로 묶어야 정상적으로 동작합니다. ios는 원인이 어찌됐던 앱을 못 열게 될 경우 launch 함수가 false를 리턴하는 것을 확인했기 떄문에 canLaunch를 확인하는 것이 별로 의미가 없다고 판단해 제거했습니다. 따라서 ios는 그대로 유지하고 안드로이드는 canLaunch를 확인하지 않는 대신 try catch로 변경해주시면 됩니다.

@syonkr
Copy link
Contributor Author

syonkr commented Jan 4, 2024

말씀주신 부분 관련하여 코드 반영하여 적용하였습니다.

url_launcher_ios
url_launcher_android

ios같은경우 SafariViewLoadFail, invalidUrl 이 아닌이상 Excpetion이 일어날일이 없는 반면 android에서는 succeeded가 아닐시 무조건 PlatformException를 throw 하는것을 확인하였습니다.

말씀주신대로 처리하여도 문제는 없을것으로 예상되지만 ios 같은경우에도 Exception이 발생할 수 있다는것이 마음에 걸리네요

@syonkr
Copy link
Contributor Author

syonkr commented Jan 4, 2024

ios에서 Exception이 무조건 발생하지 않는다는 보장이 어려울뿐더러, aos ios 상관없이 통합으로 처리해도 문제 없을것으로 보여 코드 수정해두었습니다.

Copy link
Contributor

@anymate98 anymate98 left a comment

Choose a reason for hiding this comment

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

예상되는 문제점 지적해주셔서 감사합니다!

@anymate98 anymate98 merged commit 187ced4 into iamport:main Jan 5, 2024
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.

None yet

2 participants