add wallet overview #16855

erikseppanen · 2023-08-03T02:34:24Z

Note: this uses a fake network dropdown component (the empty rounded rectangle above), as a placeholder until the real one is ready.

status: ready

status-im-auto · 2023-08-03T02:40:27Z

Jenkins Builds

Click to see older builds (8)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`a5a1c43`	#1	2023-08-03 02:40:27	~5 min	`android-e2e`	🤖`apk` 📲
✔️	`a5a1c43`	#1	2023-08-03 02:42:48	~8 min	`android`	🤖`apk` 📲
✔️	`a5a1c43`	#1	2023-08-03 02:44:06	~9 min	`tests`	📄`log`
✔️	`a5a1c43`	#1	2023-08-03 02:45:05	~10 min	`ios`	📱`ipa` 📲

✔️	`219ff74`	#2	2023-08-03 15:41:42	~5 min	`android-e2e`	🤖`apk` 📲
✔️	`219ff74`	#2	2023-08-03 15:43:45	~7 min	`ios`	📱`ipa` 📲
✔️	`219ff74`	#2	2023-08-03 15:45:32	~9 min	`android`	🤖`apk` 📲
✔️	`219ff74`	#2	2023-08-03 15:46:52	~10 min	`tests`	📄`log`

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`91b549e`	#3	2023-08-03 18:50:05	~5 min	`android-e2e`	🤖`apk` 📲
✔️	`91b549e`	#3	2023-08-03 18:50:09	~5 min	`android`	🤖`apk` 📲
✔️	`91b549e`	#3	2023-08-03 18:52:13	~7 min	`ios`	📱`ipa` 📲
✔️	`91b549e`	#3	2023-08-03 18:52:39	~8 min	`tests`	📄`log`

✔️	`0119120`	#4	2023-08-04 14:18:17	~6 min	`android`	🤖`apk` 📲
✔️	`0119120`	#4	2023-08-04 14:18:21	~6 min	`android-e2e`	🤖`apk` 📲
✔️	`0119120`	#4	2023-08-04 14:20:11	~8 min	`ios`	📱`ipa` 📲
✔️	`0119120`	#5	2023-08-04 15:40:52	~8 min	`tests`	📄`log`

ibrkhalil · 2023-08-03T06:55:35Z

src/quo2/components/wallet/wallet_overview/view.cljs

+           [rn/view {:style (style/loading-bar 32 10 8 theme)}]
+           [rn/view {:style (style/loading-bar 32 10 4 theme)}]
+           [rn/view {:style (style/loading-bar 62 10 4 theme)}]
+           [rn/view {:style (style/loading-bar 10 10 0 theme)}]])


I think the props to loading-bar will be more readable if they were a map

Thanks for the review @ibrkhalil!

done

ibrkhalil · 2023-08-03T06:56:04Z

src/quo2/components/wallet/wallet_overview/view.cljs

+            end-date]])
+
+        (when (not (#{:none :selected} time-frame))
+


Unneeded line break

ibrkhalil · 2023-08-03T06:59:39Z

src/quo2/components/wallet/wallet_overview/view.cljs

+        {:style {:flex-direction  :row
+                 :flex            1
+                 :justify-content :space-between
+                 :align-items     :center}}


Can these live inside style.cljs?

ibrkhalil · 2023-08-03T06:59:50Z

src/quo2/components/wallet/wallet_overview/view.cljs

+        [network-dropdown]]
+
+       [rn/view {:style style/container-metrics}
+


Unneeded line break

ibrkhalil · 2023-08-03T07:00:12Z

src/quo2/components/wallet/wallet_overview/view.cljs

+    {:style {:border-color  colors/neutral-50
+             :border-width  1
+             :border-radius 10
+             :width         68
+             :height        32}}]])


Can these live inside style.cljs?

ibrkhalil · 2023-08-03T07:00:41Z

src/quo2/components/wallet/wallet_overview/view.cljs

+             :style  {:color        (colors/theme-colors colors/neutral-80-opa-40
+                                                         colors/white-opa-40
+                                                         theme)
+                      :margin-right 8}}


Can these live inside style.cljs?

ibrkhalil · 2023-08-03T07:02:45Z

src/quo2/components/wallet/wallet_overview/view.cljs

+        (when (= state :loading)
+          [rn/view
+           {:style {:flex-direction :row
+                    :align-items    :center}}
+           [rn/view {:style (style/loading-bar 32 10 8 theme)}]
+           [rn/view {:style (style/loading-bar 32 10 4 theme)}]
+           [rn/view {:style (style/loading-bar 62 10 4 theme)}]
+           [rn/view {:style (style/loading-bar 10 10 0 theme)}]])


Can we extract this into a separate function?
I think it'll shorten this function's height, which make it more readable.

J-Son89 · 2023-08-03T07:40:07Z

src/quo2/core.cljs

@@ -105,7 +105,8 @@
    quo2.components.text-combinations.title.view
    quo2.components.wallet.network-amount.view
    quo2.components.wallet.network-bridge.view
-    quo2.components.wallet.account-card.view))
+    quo2.components.wallet.account-card.view


let's keep these imports alpha sorted, ah it was not your changes that did this. yet if you could put quo2.components.wallet.account-card.view in order it would be great 👍

J-Son89 · 2023-08-03T07:43:03Z

src/status_im2/contexts/quo_preview/wallet/wallet_overview.cljs

+   {:background-color (colors/theme-colors colors/white
+                                           colors/neutral-95)
+    :flex             1}
+   [rn/flat-list


as @smohamedjavid said in some other pr's. Can we move away from this flat list approach? - Imo I do not know why we are using it here. cant' we just render the components in a normal structure? - curious if anyone knows what benefits do we gain by doing this?

Thanks @J-Son89 for the review!

I'm using the boilerplate quo-preview code, as you know. If we decide to change that convention, I have no objections to following that convention.

As a side note, I'd probably find it useful to generate all or some of the common 'figma permutations' on one screen. Sometimes I change code for one permutation, but it unexpectedly adversely affects another one, which I find out later when I specifically look at that one. And at that point, I may have change many things so I have to figure out what caused it. If they were displayed at the same time on the same screen, I'd notice it immediately. Of course, there is only so much screen to display for components, but for this one at least, 5 of them might fit. Just a suggestion, maybe related to flat-list.

I agree Erik, we could even have 2 views - one variant and all variants. I think we are going to gather feedback on what features could be useful here.

And yeah you're right, we can leave it for now until a convention is decided 👌

J-Son89 · 2023-08-03T12:42:41Z

src/quo2/components/wallet/wallet_overview/style.cljs

+   :margin-horizontal 4
+   :width             2
+   :height            2
+   :border-radius     200})


I think border radius can be a bit less :)

yes, definitely :)

done

OmarBasem

I think the component tree of view-internal is a bit big and can be broken down into smaller parts. Other than that LGTM! 👍

OmarBasem · 2023-08-03T16:59:23Z

src/quo2/components/wallet/wallet_overview/view.cljs

+(defn- view-internal
+  [{:keys [state time-frame metrics balance date begin-date end-date
+           currency-change percentage-change theme]}]
+  [rn/view {:style style/container-info}
+   [rn/view {:style {:flex 1}}
+    [rn/view {:style style/container-balance}
+     (if (= state :loading)
+       (loading-bars [{:width 201 :height 20 :margin 0}] theme)
+       [quo2/text
+        {:weight :semi-bold
+         :size   :heading-1
+         :style  (style/style-text-header theme)}
+        balance])
+     [network-dropdown]]
+    [rn/view {:flex-direction :row}
+     (when (= state :loading)
+       [rn/view
+        {:style {:flex-direction :row
+                 :align-items    :center}}
+        (loading-bars [{:width 32 :height 10 :margin 8}
+                       {:width 32 :height 10 :margin 4}
+                       {:width 62 :height 10 :margin 4}
+                       {:width 10 :height 10 :margin 0}]
+                      theme)])
+     (when (= time-frame :selected)
+       [quo2/text
+        {:weight :medium
+         :size   :paragraph-2
+         :style  (style/style-text-paragraph theme)}
+        date])
+     (when (= time-frame :custom)
+       [rn/view {:style {:flex-direction :row}}
+        [quo2/text
+         {:weight :medium
+          :size   :paragraph-2
+          :style  (style/style-text-paragraph theme)}
+         begin-date]
+        [icons/icon :i/positive-right
+         {:color (style/color-text-paragraph theme)
+          :size  16}]
+        [quo2/text
+         {:weight :medium
+          :size   :paragraph-2
+          :style  (style/style-text-paragraph theme)}
+         end-date]])
+     (when (not (#{:none :selected} time-frame))
+       [rn/view {:style {:flex-direction :row}}
+        [quo2/text
+         {:weight :medium
+          :size   :paragraph-2
+          :style  {:color        (style/color-text-paragraph theme)
+                   :margin-right 8}}
+         (time-frame time-frames)]
+        (when (not= metrics :none)
+          [rn/view
+           {:style {:flex-direction :row
+                    :align-items    :center}}
+           [quo2/text
+            {:weight :medium
+             :size   :paragraph-2
+             :style  {:color (style/color-metrics metrics)}}
+            percentage-change]
+           [rn/view
+            {:style (style/dot-separator metrics)}]
+           [quo2/text
+            {:weight :medium
+             :size   :paragraph-2
+             :style  {:color        (style/color-metrics metrics)
+                      :margin-right 4}}
+            currency-change]
+           [icons/icon
+            (if (= metrics :positive) :i/positive :i/negative)
+            {:color (style/color-metrics metrics)
+             :size  16}]])])]]])
+


I think this component can be broken down into smaller components

Thanks for your review @OmarBasem!

done

briansztamfater · 2023-08-03T17:16:23Z

Hi @erikseppanen, thanks for your great work! Would be great to add some screenshots to the PR description so we can see a preview of the work.

vkjr · 2023-08-03T17:26:01Z

Hi!
It looks like part of the component wasn't covered by (if loading?) selector. Otherwise looks good to me! Thanks)

erikseppanen · 2023-08-03T18:46:33Z

Hi! It looks like part of the component wasn't covered by (if loading?) selector. Otherwise looks good to me! Thanks)

Thanks for your review @vkjr!

fixed/done

erikseppanen · 2023-08-03T19:33:40Z

@Francesca-G Appreciate your review of this component

Francesca-G

Here's the Figma frame with the design review

erikseppanen · 2023-08-04T14:54:37Z

Here's the Figma frame with the design review

Thank you @Francesca-G!

design review issue added

flexsurfer assigned erikseppanen Aug 3, 2023

ibrkhalil reviewed Aug 3, 2023

View reviewed changes

J-Son89 requested review from briansztamfater, smohamedjavid, OmarBasem and vkjr August 3, 2023 07:37

J-Son89 reviewed Aug 3, 2023

View reviewed changes

erikseppanen force-pushed the esep/wallet-overview branch from a5a1c43 to 219ff74 Compare August 3, 2023 15:35

OmarBasem approved these changes Aug 3, 2023

View reviewed changes

erikseppanen force-pushed the esep/wallet-overview branch from 219ff74 to 91b549e Compare August 3, 2023 18:44

Francesca-G reviewed Aug 4, 2023

View reviewed changes

Francesca-G added the followup-required label Aug 4, 2023

vkjr approved these changes Aug 4, 2023

View reviewed changes

add wallet overview

0119120

erikseppanen force-pushed the esep/wallet-overview branch from 91b549e to 0119120 Compare August 4, 2023 14:11

erikseppanen merged commit 2819c20 into develop Aug 4, 2023
2 checks passed

erikseppanen deleted the esep/wallet-overview branch August 4, 2023 15:43

mmilad75 pushed a commit that referenced this pull request Aug 7, 2023

Quo2 wallet: add wallet overview (#16855)

da096b0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add wallet overview #16855

add wallet overview #16855

erikseppanen commented Aug 3, 2023 •

edited

Loading

status-im-auto commented Aug 3, 2023 •

edited

Loading

ibrkhalil Aug 3, 2023

erikseppanen Aug 3, 2023

ibrkhalil Aug 3, 2023

erikseppanen Aug 3, 2023

ibrkhalil Aug 3, 2023

erikseppanen Aug 3, 2023

ibrkhalil Aug 3, 2023

erikseppanen Aug 3, 2023

ibrkhalil Aug 3, 2023

erikseppanen Aug 3, 2023

ibrkhalil Aug 3, 2023

erikseppanen Aug 3, 2023

ibrkhalil Aug 3, 2023

erikseppanen Aug 3, 2023

J-Son89 Aug 3, 2023 •

edited

Loading

erikseppanen Aug 3, 2023

J-Son89 Aug 3, 2023

erikseppanen Aug 3, 2023 •

edited

Loading

J-Son89 Aug 3, 2023

J-Son89 Aug 3, 2023

erikseppanen Aug 3, 2023 •

edited

Loading

OmarBasem left a comment

OmarBasem Aug 3, 2023

erikseppanen Aug 3, 2023

briansztamfater commented Aug 3, 2023

vkjr commented Aug 3, 2023 •

edited

Loading

erikseppanen commented Aug 3, 2023

erikseppanen commented Aug 3, 2023

Francesca-G left a comment

erikseppanen commented Aug 4, 2023 •

edited

Loading

		[network-dropdown]]

		[rn/view {:style style/container-metrics}

add wallet overview #16855

add wallet overview #16855

Conversation

erikseppanen commented Aug 3, 2023 • edited Loading

status-im-auto commented Aug 3, 2023 • edited Loading

Jenkins Builds

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

J-Son89 Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikseppanen Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikseppanen Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

OmarBasem left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briansztamfater commented Aug 3, 2023

vkjr commented Aug 3, 2023 • edited Loading

erikseppanen commented Aug 3, 2023

erikseppanen commented Aug 3, 2023

Francesca-G left a comment

Choose a reason for hiding this comment

erikseppanen commented Aug 4, 2023 • edited Loading

erikseppanen commented Aug 3, 2023 •

edited

Loading

status-im-auto commented Aug 3, 2023 •

edited

Loading

J-Son89 Aug 3, 2023 •

edited

Loading

erikseppanen Aug 3, 2023 •

edited

Loading

erikseppanen Aug 3, 2023 •

edited

Loading

vkjr commented Aug 3, 2023 •

edited

Loading

erikseppanen commented Aug 4, 2023 •

edited

Loading