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

Wallet - refresh activity after sending a transaction #19984

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

J-Son89
Copy link
Member

@J-Son89 J-Son89 commented May 10, 2024

fixes: #19981, #19519

This pr does a few things

  • corrects the navigation for MVP, i.e after confirming a transaction it goes directly to account view and shows the activity tab

  • refreshes activity after sending a transaction

  • this in turn refreshes the recent transactions tab in the select address page of sending flows.

  • to make this work I had to update the tabs component to be controllable, as such we should check other pages with tabs are working alright.

Screen.Recording.2024-05-10.at.18.28.24.mov

@status-im-auto
Copy link
Member

status-im-auto commented May 10, 2024

Jenkins Builds

Click to see older builds (20)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 7904672 #1 2024-05-10 17:42:11 ~4 min tests 📄log
✔️ 7904672 #1 2024-05-10 17:47:04 ~9 min ios 📱ipa 📲
✔️ 7904672 #1 2024-05-10 17:47:54 ~10 min android-e2e 🤖apk 📲
✔️ 7904672 #1 2024-05-10 17:48:03 ~10 min android 🤖apk 📲
4ba54ed #2 2024-05-15 07:46:50 ~2 min tests 📄log
✔️ 4ba54ed #2 2024-05-15 07:52:17 ~8 min android-e2e 🤖apk 📲
✔️ 4ba54ed #2 2024-05-15 07:52:20 ~8 min android 🤖apk 📲
✔️ 4ba54ed #2 2024-05-15 07:53:40 ~9 min ios 📱ipa 📲
82bc6d5 #4 2024-05-21 00:14:23 ~2 min tests 📄log
✔️ 82bc6d5 #4 2024-05-21 00:18:31 ~6 min android-e2e 🤖apk 📲
✔️ 82bc6d5 #4 2024-05-21 00:21:25 ~9 min ios 📱ipa 📲
✔️ 82bc6d5 #4 2024-05-21 00:21:50 ~10 min android 🤖apk 📲
c98c966 #5 2024-05-21 13:03:00 ~4 min tests 📄log
✔️ c98c966 #5 2024-05-21 13:08:23 ~9 min android 🤖apk 📲
✔️ c98c966 #5 2024-05-21 13:08:28 ~9 min android-e2e 🤖apk 📲
✔️ c98c966 #5 2024-05-21 13:09:11 ~10 min ios 📱ipa 📲
✔️ 8d1934c #6 2024-05-21 17:32:24 ~9 min ios 📱ipa 📲
✔️ 8d1934c #6 2024-05-21 17:33:35 ~10 min android 🤖apk 📲
✔️ 8d1934c #6 2024-05-21 17:33:35 ~10 min android-e2e 🤖apk 📲
✔️ 8d1934c #7 2024-05-21 18:13:01 ~3 min tests 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 21e527a #8 2024-05-23 21:00:00 ~3 min tests 📄log
✔️ 21e527a #7 2024-05-23 21:05:48 ~9 min ios 📱ipa 📲
✔️ 21e527a #7 2024-05-23 21:06:52 ~10 min android-e2e 🤖apk 📲
✔️ 21e527a #7 2024-05-23 21:07:00 ~10 min android 🤖apk 📲
✔️ d503f86 #9 2024-05-23 22:37:37 ~3 min tests 📄log
✔️ d503f86 #8 2024-05-23 22:43:13 ~9 min ios 📱ipa 📲
✔️ d503f86 #8 2024-05-23 22:44:31 ~10 min android-e2e 🤖apk 📲
✔️ d503f86 #8 2024-05-23 22:44:33 ~10 min android 🤖apk 📲

@J-Son89 J-Son89 force-pushed the jc/refresh-activity-after-send branch from 7904672 to 4ba54ed Compare May 15, 2024 07:43
@J-Son89 J-Son89 force-pushed the jc/refresh-activity-after-send branch from 4ba54ed to e7c911f Compare May 21, 2024 00:10
@J-Son89 J-Son89 marked this pull request as ready for review May 21, 2024 12:58
@J-Son89 J-Son89 force-pushed the jc/refresh-activity-after-send branch from c98c966 to 8d1934c Compare May 21, 2024 17:22
- `scroll-on-press?` When non-nil, clicking on a tag centers it the middle
(with animation enabled).
"
(defn view-internal
Copy link
Member Author

Choose a reason for hiding this comment

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

added a malli schema and removed doc string

Copy link
Member

Choose a reason for hiding this comment

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

defn- view-internal

[{:keys [default-active data fade-end-percentage fade-end? on-change on-scroll scroll-on-press?
scrollable? style container-style size blur? in-scroll-view? customization-color]
scrollable? style container-style size blur? in-scroll-view? customization-color
active-tab-id]
Copy link
Member Author

Choose a reason for hiding this comment

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

tabs component now is controllable from outside with a prop for value.

:customization-color customization-color
:label (i18n/label :t/jump-to)}}
style/shell-button]]))))
(let [selected-tab (or (rf/sub [:wallet/account-tab]) first-tab-id)
Copy link
Member Author

Choose a reason for hiding this comment

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

selected tab is stored in the reframe db so we can go to the right tab after certain actions (e.g complete transaction)

- `scroll-on-press?` When non-nil, clicking on a tag centers it the middle
(with animation enabled).
"
(defn view-internal
Copy link
Member

Choose a reason for hiding this comment

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

defn- view-internal

Comment on lines -413 to +431
[:wallet/wizard-navigate-forward
{:current-screen :screen/wallet.transaction-confirmation
:flow-id :wallet-send-flow}]]]})))
[:wallet/end-transaction-flow]]]})))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we update the flow_config of bridge and send?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes good point, I'll double check that. I see you had it in the similar pr so will check with that too 🙏

Copy link
Member

@smohamedjavid smohamedjavid left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@@ -0,0 +1,32 @@
(ns quo.components.tabs.tabs.schema)

(def ^:private ?data-schema
Copy link
Contributor

Choose a reason for hiding this comment

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

The question mark prefix should be enough to tell ?data is a schema.

:default-active first-tab-id
:active-tab-id selected-tab
:data (tabs-data watch-only?)
:on-change #(rf/dispatch [:wallet/select-account-tab %])
Copy link
Contributor

Choose a reason for hiding this comment

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

The handler can be extracted to a var because it doesn't close over values.

@@ -52,6 +56,7 @@
(rf/reg-event-fx :wallet/close-account-page
(fn [_]
{:fx [[:dispatch [:wallet/clean-current-viewing-account]]
[:dispatch [:wallet/select-account-tab nil]]
Copy link
Contributor

Choose a reason for hiding this comment

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

A minor suggestion, but I'd have a separate event to deselect the account tab instead of relying on nil.

(rf/reg-event-fx :wallet/deselect-account-tab
 (fn [{:keys [db]} _]
   {:db (update-in db [:wallet :ui :account-page] dissoc :active-tab)}))

Copy link
Member Author

Choose a reason for hiding this comment

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

done

[:sequential
[:maybe
[:map
[:id [:maybe :keyword]]
Copy link
Contributor

Choose a reason for hiding this comment

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

About the :maybe here, wouldn't it be a bug to have a tab item without an ID? Seems really odd to rely on a nil ID, but maybe that's how it already is and the schema is just explicitly telling us now this oddity?

[:catn
[:props
[:map
[:default-active {:optional true} [:maybe :keyword]]
Copy link
Contributor

Choose a reason for hiding this comment

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

When opening the AC, the schema fails because we pass :default-active as a number, not as a keyword.

Copy link
Member Author

Choose a reason for hiding this comment

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

I forgot about AC, will update

@J-Son89 J-Son89 force-pushed the jc/refresh-activity-after-send branch from 8d1934c to 21e527a Compare May 23, 2024 20:55
@J-Son89 J-Son89 force-pushed the jc/refresh-activity-after-send branch from 21e527a to d503f86 Compare May 23, 2024 22:33
Copy link
Contributor

@ilmotta ilmotta left a comment

Choose a reason for hiding this comment

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

PR LGTM! And thanks for adding schemas to the complex tabs component 👏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Pipeline for QA
  
REVIEW
Development

Successfully merging this pull request may close these issues.

Wallet- Update Activity After Sending a transaction
6 participants