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

FFFra/account card component #16309

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/quo2/components/wallet/account_card/component_spec.cljs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
(ns quo2.components.wallet.account-card.component-spec
(:require [quo2.components.wallet.account-card.view :as account-card]
[test-helpers.component :as h]))

(def john-doe-name "John Doe")

(defn get-test-data
[type watch-only?]
{:name john-doe-name
:balance "€1,000.00"
:percentage-value "50%"
:amount "€500.00"
:customization-color :blue
:watch-only? watch-only?
:type type})

(h/describe "Account_card tests"
(h/test "Renders normal"
(let [data (get-test-data :default false)]
(h/render [account-card/view data])
(h/is-truthy (h/get-by-text john-doe-name))))

(h/test "Renders watch-only"
(let [data (get-test-data :watch-only? true)]
(h/render [account-card/view data])
(h/is-truthy (h/get-by-text john-doe-name))))

(h/test "Renders add-account"
(let [data {:type :add-account}]
(h/render [account-card/view data])
(h/is-truthy (h/get-by-label-text :plus-button))))

(h/test "Add account on press fires correctly"
(let [handler (h/mock-fn)
data {:type :add-account
:handler handler}]
(h/render [account-card/view data])
(h/fire-event :on-press (h/get-by-label-text :plus-button))
(h/was-called handler))))
90 changes: 90 additions & 0 deletions src/quo2/components/wallet/account_card/style.cljs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
(ns quo2.components.wallet.account-card.style
(:require
[quo2.foundations.colors :as colors]))

(defn text-color
[watch-only?]
(if (and watch-only? (not (colors/dark?)))
colors/neutral-100
colors/white))

(defn card
[customization-color watch-only?]
{:width 161
:height 88
:background-color (if watch-only?
(colors/theme-colors colors/neutral-80-opa-5 colors/neutral-95)
(colors/custom-color-by-theme customization-color 50 60))
:border-radius 16
:border-width 1
:border-color (if watch-only?
(colors/theme-colors colors/neutral-80-opa-5 colors/white-opa-5)
(colors/custom-color-by-theme customization-color 50 60))
:padding-horizontal 12
:padding-top 8
:padding-bottom 10})

(def profile-container
{:margin-bottom 8
:flex-direction :row})

(def metrics-container
{:flex-direction :row
:align-items :center})

(def title
{:margin-bottom 9})

(defn account-name
[watch-only?]
{:style {:color (text-color watch-only?)
:margin-left 5}
:size :paragraph-2
Copy link
Contributor

Choose a reason for hiding this comment

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

These are not style properties. Please move size and weight outside this namespace. Vars metrics, account-value and add-account-title also should be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

:weight :medium})

(def watch-only-container
{:flex-direction :row
:justify-content :space-between
:align-items :center
:flex 1})

(defn account-value
[watch-only?]
{:style {:color (text-color watch-only?)}
:size :heading-2
:weight :semi-bold})

(defn metrics
[watch-only?]
{:weight :semi-bold
:size :paragraph-2
:style {:color (if (and watch-only? (not (colors/dark?)))
colors/neutral-80-opa-60
colors/white-opa-70)}})

(defn separator
[watch-only?]
{:width 1
:height 10
:border-width 1
:border-color (if watch-only?
colors/neutral-80-opa-20
colors/white-opa-40)
:margin-horizontal 4})

(def add-account-container
{:width 161
:height 88
:border-color (colors/theme-colors colors/neutral-20 colors/white-opa-5)
Copy link
Contributor

Choose a reason for hiding this comment

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

If the theme changes the map won't be updated because colors/theme-colors relies on a reactive atom.

Copy link
Contributor

Choose a reason for hiding this comment

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

that is can you make this a defn. e,g

(defn [] add-account-container
  {:width              161
   :height             88
   ...

Copy link
Contributor

Choose a reason for hiding this comment

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

This call to colors/theme-colors also need to be fixed to accept a theme argument.

:border-width 1
:border-style :dashed
:align-items :center
:justify-content :center
:border-radius 16
:padding-vertical 12
:padding-horizontal 10})

(def add-account-title
{:size :paragraph-2
:weight :medium
:margin-top 4})
50 changes: 50 additions & 0 deletions src/quo2/components/wallet/account_card/view.cljs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
(ns quo2.components.wallet.account-card.view
(:require [react-native.core :as rn]
[quo2.foundations.colors :as colors]
[quo2.components.icon :as icon]
[quo2.components.wallet.account-card.style :as style]
[status-im2.common.plus-button.view :as plus-button]
[utils.i18n :as i18n]
[quo2.components.markdown.text :as text]))

(defn user-account-view
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually we don't append view or component suffixes unless there's a naming conflict and the function name would shadow a parameter. So just user-account is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, nice catch

[{:keys [name balance percentage-value amount customization-color type]}]
(let [watch-only? (= :watch-only type)]
[:<>
[rn/view (style/card customization-color watch-only?)
[rn/view style/profile-container
[icon/icon :contact]
[rn/view style/watch-only-container
[text/text (style/account-name watch-only?)
name]
(when watch-only? [icon/icon :reveal {:color colors/neutral-50 :size 12}])]]

[text/text (style/account-value watch-only?) balance]
[rn/view style/metrics-container
[rn/view {:margin-right 5.5}
[icon/icon :positive
{:color (if (and watch-only? (not (colors/dark?)))
colors/neutral-100
colors/white)
:size 16}]]
[text/text (style/metrics watch-only?) percentage-value]
[rn/view (style/separator watch-only?)]
[text/text (style/metrics watch-only?) amount]]]]))

(defn add-account-view
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, make all vars private, except view. For defn we use defn- (not the metadata ^:private).

[{:keys [handler type add-account customization-color]}]
[rn/view style/add-account-container
[plus-button/plus-button
{:on-press handler
:customization-color customization-color
}]
Copy link
Contributor

Choose a reason for hiding this comment

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

parentheses

[text/text style/add-account-title (i18n/label :t/add-account)]])

(defn view
Copy link
Contributor

Choose a reason for hiding this comment

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

We are gradually making DS components theme aware by using quo2.theme/with-theme. You can find some examples already, e.g. quo2.components.dividers.divider-label.

The way it works is that the HoF passes the current theme set by a React Context, and then in the component you use this theme parameter whenever you need to use a color or anything that depends on the current theme. In practice, almost all calls to colors/theme-colors should pass a third argument theme (the current theme).

So there are some changes you need to make in this file and in the style.cljs one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all done =)

[{:keys [type] :as props}]
(case type
:watch-only [user-account-view props]
:add-account [add-account-view props]
:default [user-account-view
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can be joined with the next one to look like [user-account-view props]

props]
nil))
4 changes: 4 additions & 0 deletions src/quo2/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
quo2.components.tags.token-tag
quo2.components.text-combinations.title.view
quo2.components.settings.settings-list.view
quo2.components.wallet.account-card.view
quo2.components.settings.reorder-item.view))

(def text quo2.components.markdown.text/text)
Expand Down Expand Up @@ -236,3 +237,6 @@
(def url-preview quo2.components.links.url-preview.view/view)
(def url-preview-list quo2.components.links.url-preview-list.view/view)
(def link-preview quo2.components.links.link-preview.view/view)

;;;; WALLET
(def account-card quo2.components.wallet.account-card.view/view)
3 changes: 2 additions & 1 deletion src/quo2/core_spec.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@
[quo2.components.settings.reorder-item.component-spec]
[quo2.components.settings.settings-list.component-spec]
[quo2.components.share.share-qr-code.component-spec]
[quo2.components.tags.--tests--.status-tags-component-spec]))
[quo2.components.tags.--tests--.status-tags-component-spec]
[quo2.components.wallet.account-card.component-spec]))
11 changes: 8 additions & 3 deletions src/status_im2/contexts/quo_preview/main.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@
[status-im2.contexts.quo-preview.wallet.network-amount :as network-amount]
[status-im2.contexts.quo-preview.wallet.network-breakdown :as network-breakdown]
[status-im2.contexts.quo-preview.wallet.token-overview :as token-overview]
[status-im2.contexts.quo-preview.keycard.keycard :as keycard]))
[status-im2.contexts.quo-preview.keycard.keycard :as keycard]
[status-im2.contexts.quo-preview.wallet.account-card :as account-card]))

(def screens-categories
{:foundations [{:name :shadows
Expand Down Expand Up @@ -355,7 +356,10 @@
:text-combinations [{:name :title
:options {:topBar {:visible true}}
:component title/preview-title}]
:wallet [{:name :lowest-price
:wallet [{:name :account-card
:options {:topBar {:visible true}}
:component account-card/preview-account-card}
{:name :lowest-price
:options {:topBar {:visible true}}
:component lowest-price/preview-lowest-price}
{:name :token-overview
Expand All @@ -369,7 +373,8 @@
:component network-amount/preview}]
:keycard [{:name :keycard-component
:options {:topBar {:visible true}}
:component keycard/preview-keycard}]})
:component keycard/preview-keycard}]
})

(def screens (flatten (map val screens-categories)))

Expand Down
56 changes: 56 additions & 0 deletions src/status_im2/contexts/quo_preview/wallet/account_card.cljs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
(ns status-im2.contexts.quo-preview.wallet.account-card
(:require [react-native.core :as rn]
[quo2.foundations.colors :as colors]
[quo2.components.markdown.text :as text]
[quo2.core :as quo]))

(def mock-data
[{:id 1
:name "Alisher account"
:balance "€2,269.12"
:percentage-value "16.9%"
:amount "€570.24"
:customization-color :army
:type :default}
{:id 2
:name "Ben’s fortune"
:balance "€2,269.12"
:percentage-value "16.9%"
:amount "€570.24"
:watch-only? true
:type :watch-only}
{:id 3
:type :add-account
:customization-color :blue
:handler #(js/alert "Add account pressed")}])

(defn- separator
[]
[rn/view {:style {:width 40}}])

(defn cool-preview
[]
[rn/view
{:style {:margin-vertical 40
:margin-left 40
:flex 1}}
[text/text {:size :heading-1 :weight :semi-bold :style {:margin-bottom 40}} "Account card"]
[rn/flat-list
{:data mock-data
:key-extractor #(str (:id %))
Copy link
Contributor

Choose a reason for hiding this comment

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

just :d will do

:key-extractor :id

since :id is already a function as it implementsiFn https://cljs.github.io/api/cljs.core/IFn
and I think id is already a string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tks! I will apply it if we decide to also include the flat list to the variants preview

:horizontal true
:separator [separator]
:render-fn quo/account-card
:showsHorizontalScrollIndicator false}]])
Copy link
Contributor

Choose a reason for hiding this comment

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

The keyword should use kebab case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was removed due to new implementation. But you are right =)


(defn preview-account-card
[]
[rn/view
{:background-color (colors/theme-colors colors/white
colors/neutral-90)
:flex 1}
[rn/flat-list
{:flex 1
:keyboard-should-persist-taps :always
:header [cool-preview]
:key-fn str}]])