Skip to content

Wc debugging#935

Merged
lmcmz merged 20 commits intodevelopfrom
wc-debugging
May 2, 2025
Merged

Wc debugging#935
lmcmz merged 20 commits intodevelopfrom
wc-debugging

Conversation

@lealobanov
Copy link
Copy Markdown
Contributor

Related Issue

Closes #???

Summary of Changes

Need Regression Testing

  • Yes
  • No

Risk Assessment

  • Low
  • Medium
  • High

Additional Notes

Screenshots (if applicable)

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 2, 2025

PR Summary

Improved the WalletConnect session approval flow by adding better browser state detection and toast notification logic. The changes ensure users are properly notified to return to the browser only when necessary, avoiding redundant notifications when using in-app browser authentication.

Changes

File Summary
app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectDelegate.kt Updated session approval logic to check both browser container visibility and browser instance state. Toast notification for returning to browser now only shows when browser is not active and no redirect URL exists. Added improved logging for debugging.

autogenerated by presubmit.ai

@lealobanov lealobanov marked this pull request as ready for review May 2, 2025 06:33
@lealobanov lealobanov requested a review from a team as a code owner May 2, 2025 06:33
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

🚨 Pull request needs attention.

Review Summary

Commits Considered (20)
  • ce818e9: Merge remote-tracking branch 'origin/wc-debugging' into wc-debugging
  • 83e4d1c: Debugging
  • c6f7acd: Merge branch 'develop' into wc-debugging
  • be5e55c: Debugging
  • adbe316: Debug build.gradle
  • 5554f92: Debug build.gradle
  • e892a05: Debug build.gradle
  • 541c5a7: Debug build.gradle
  • d6c87f4: Debug build.gradle
  • 955d8eb: Merge remote-tracking branch 'origin/wc-debugging' into wc-debugging
  • 0d1c3b1: Debug build.gradle
  • 31bb763: Merge branch 'develop' into wc-debugging
  • 5714237: Debug build.gradle
  • 010aa2d: Hide toast when on inapp browser auth
  • 9ec01b4: Clean up
  • a179ec4: WC redirect fallback
  • 5dcc910: WC redirect fallback
  • 3b0743c: WC redirect fallback
  • 9e26de8: WC redirect fallback
  • 79de9db: WIP - integration
Files Processed (1)
  • app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectDelegate.kt (2 hunks)
Actionable Comments (1)
  • app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectDelegate.kt [184-185]

    possible bug: "Potential null pointer exception in browser state check"

Skipped Comments (0)

Comment on lines +184 to +185
val browserInstance = browserInstance()
if (browserContainer?.visibility != View.VISIBLE || browserInstance == null) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The browserInstance() call result is not checked for null before accessing browserContainer?.visibility. If browserContainer is not null but browserInstance is null, the condition will still evaluate the second part of the OR expression, which could lead to unexpected behavior. Consider restructuring the condition to check both nullable values safely:

val browserContainer = WindowFrame.browserContainer()
val browserInstance = browserInstance()
if (browserContainer?.visibility != View.VISIBLE || browserInstance == null) {

@lmcmz lmcmz merged commit d216f43 into develop May 2, 2025
3 checks passed
@lmcmz lmcmz deleted the wc-debugging branch May 2, 2025 13:20
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.

2 participants