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

chore: dispatch open-url event instead of calling function directly #21246

Merged
merged 2 commits into from
Sep 20, 2024

Conversation

seanstrom
Copy link
Member

@seanstrom seanstrom commented Sep 10, 2024

fixes #20377
follow-up pr: #20752

Summary

  • This PR attempts to replace usage of rn/open-url (for opening links) to rf/dispatch with the :open-url event
    • This PR also defines the :open-url Re-Frame event

Platforms

  • Android
  • iOS

Areas that maybe impacted

Functional
  • Opening links from the buy crypto bottom-sheet
  • Opening the read-more link when creating an account

Steps to test

  • The steps to verify the behaviour of these changes are captured in the screen recordings below
  • Though this steps assume that the user has opened the Status mobile app and has an account with a balance greater than 0.

Before and after screenshots comparison

After Changes

Opening links from the buy crypto bottom-sheet
Screen.Recording.2024-09-10.at.17.06.43.mov
Opening the read-more link from the account creation info
Screen.Recording.2024-09-10.at.17.07.36.mov

status: ready

@status-im-auto
Copy link
Member

status-im-auto commented Sep 10, 2024

Jenkins Builds

Click to see older builds (7)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 2165493 #1 2024-09-10 16:11:33 ~5 min tests 📄log
✔️ 2165493 #1 2024-09-10 16:14:58 ~8 min android-e2e 🤖apk 📲
✔️ 2165493 #1 2024-09-10 16:15:49 ~9 min android 🤖apk 📲
✔️ c462391 #2 2024-09-11 14:34:22 ~4 min tests 📄log
✔️ c462391 #2 2024-09-11 14:37:18 ~7 min android-e2e 🤖apk 📲
✔️ c462391 #2 2024-09-11 14:37:19 ~7 min android 🤖apk 📲
✔️ c462391 #2 2024-09-11 14:43:34 ~13 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 0992b58 #3 2024-09-16 11:08:40 ~5 min tests 📄log
✔️ 0992b58 #3 2024-09-16 11:10:10 ~6 min android-e2e 🤖apk 📲
✔️ 0992b58 #3 2024-09-16 11:10:23 ~6 min android 🤖apk 📲
✔️ 0992b58 #3 2024-09-16 11:16:13 ~12 min ios 📱ipa 📲
✔️ 692af3b #4 2024-09-20 14:24:52 ~4 min tests 📄log
✔️ 692af3b #4 2024-09-20 14:27:37 ~7 min android-e2e 🤖apk 📲
✔️ 692af3b #4 2024-09-20 14:28:03 ~7 min android 🤖apk 📲
✔️ 692af3b #4 2024-09-20 14:40:51 ~20 min ios 📱ipa 📲

src/status_im/navigation/events.cljs Outdated Show resolved Hide resolved
@status-im-auto
Copy link
Member

100% of end-end tests have passed

Total executed tests: 7
Failed tests: 0
Expected to fail tests: 0
Passed tests: 7

Passed tests (7)

Click to expand

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231
Device sessions

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
Device sessions

2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device sessions

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
2. test_wallet_send_eth, id: 727229

@pavloburykh pavloburykh self-assigned this Sep 16, 2024
@pavloburykh
Copy link
Contributor

@seanstrom thank you for the PR. Tested: mentioned links are being opened successfully. If there are no other places which need to be checked for regression - this PR is ready for merge. In case this PR could affect any other places related to urls opening, please let me know.

@seanstrom
Copy link
Member Author

@pavloburykh is it possible to test the app's dApp functionality? I think there's some features that open a url when connecting to a dApp from the status mobile app.

@pavloburykh
Copy link
Contributor

@pavloburykh is it possible to test the app's dApp functionality? I think there's some features that open a url when connecting to a dApp from the status mobile app.

@seanstrom thanks. Tested, LGTM. Ready to merge.

@seanstrom seanstrom merged commit 0f20f07 into develop Sep 20, 2024
6 checks passed
@seanstrom seanstrom deleted the seanstrom/refactor-open-url branch September 20, 2024 15:48
@seanstrom seanstrom mentioned this pull request Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Move :browser.ui/open-url to new codebase
7 participants