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.

Sign up for GitHub

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

[#20500] Fix derived accounts #20536

Merged
merged 11 commits into from
Jun 26, 2024
Merged

[#20500] Fix derived accounts #20536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
74104a7
WIp
ulisesmac Jun 20, 2024
c82e630
Fix color of non-selected keypairs
ulisesmac Jun 21, 2024
6f29573
Fix pressable area in keypairs
ulisesmac Jun 21, 2024
204bff0
Make validation more robust
ulisesmac Jun 21, 2024
9fc442e
Make confirm button always pressable
ulisesmac Jun 21, 2024
8ea2cb5
Fix wrong sentence in errors
ulisesmac Jun 21, 2024
b630eb9
Solve account creation by fixing a memoized callback
ulisesmac Jun 21, 2024
2a668c7
WIP
ulisesmac Jun 21, 2024
ccd1c59
Fix
ulisesmac Jun 21, 2024
061fe7e
Lint fix
ulisesmac Jun 21, 2024
4c226b3
Improve logging
ulisesmac Jun 21, 2024
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
22 changes: 11 additions & 11 deletions src/quo/components/buttons/slide_button/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
[react-native.reanimated :as reanimated]))

(defn drag-gesture
[x-pos disabled? track-width sliding-complete?
set-sliding-complete
on-complete reset-fn]
[x-pos disabled? track-width sliding-complete? set-sliding-complete on-complete reset-fn]
(-> (gesture/gesture-pan)
(gesture/with-test-ID :slide-button-gestures)
(gesture/enabled (not disabled?))
Expand Down Expand Up @@ -71,14 +69,16 @@
(dimensions :thumb))
[dimensions])
custom-color (if (= type :danger) :danger customization-color)
gesture (rn/use-memo #(drag-gesture x-pos
disabled?
(dimensions :usable-track)
sliding-complete?
set-sliding-complete
on-complete
reset-fn)
[sliding-complete? disabled?])]
gesture (rn/use-memo
(fn []
(drag-gesture x-pos
disabled?
(dimensions :usable-track)
sliding-complete?
set-sliding-complete
on-complete
reset-fn))
[sliding-complete? disabled? on-complete])]
ilmotta marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

This is actually interesting 🤔

[gesture/gesture-detector
{:gesture gesture}
[reanimated/view
Expand Down
16 changes: 8 additions & 8 deletions src/quo/components/wallet/keypair/style.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
[{:keys [blur? customization-color theme selected? container-style]}]
(merge {:border-radius 16
:border-width 1
:border-color (if selected?
(if blur?
colors/white
(colors/resolve-color customization-color theme))
(if blur?
colors/white-opa-5
(colors/theme-colors colors/neutral-10 colors/neutral-80 theme)))
:padding-bottom 8}
:padding-bottom 8
:border-color (cond
(and selected? blur?) colors/white
selected? (colors/resolve-color customization-color theme)
blur? colors/white-opa-5
:else (colors/theme-colors colors/neutral-10
colors/neutral-80
theme))}
Comment on lines +10 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a refactor from 2 ifs -> cond

container-style))

(def header-container
Expand Down
37 changes: 20 additions & 17 deletions src/quo/components/wallet/keypair/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,20 +95,23 @@

(defn view
[{:keys [accounts action container-style selected? on-press] :as props}]
[rn/pressable
{:style (style/container (merge props
{:selected? selected?
:container-style container-style}))
:on-press #(when (= action :selector) (on-press))}
[rn/view {:style style/header-container}
[avatar props]
[rn/view
{:style {:margin-left 8
:flex 1}}
[title-view (assoc props :selected? selected?)]
[details-view props]]]
[rn/flat-list
{:data accounts
:render-fn acc-list-card
:separator [rn/view {:style {:height 8}}]
:style {:padding-horizontal 8}}]])
(let [theme (quo.theme/use-theme)]
[rn/pressable
{:style (style/container (assoc props
:selected? selected?
:container-style container-style
:theme theme))
Comment on lines +100 to +103
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a refactor from merge -> assoc

:on-press #(when (= action :selector) (on-press))
:pointer-events :box-only}
[rn/view {:style style/header-container}
Comment on lines +105 to +106
Copy link
Member

@seanstrom seanstrom Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hey @ulisesmac 👋
Do we need pointer-events :box-only here? I think this piece of code is causing an issue when we have a button embedded inside the key-pair component. For example, the pointer events won't reach the button and the on-press callback won't be called.

[avatar props]
[rn/view
{:style {:margin-left 8
:flex 1}}
[title-view (assoc props :selected? selected?)]
[details-view props]]]
[rn/flat-list
{:data accounts
:render-fn acc-list-card
:separator [rn/view {:style {:height 8}}]
:style {:padding-horizontal 8}}]]))
15 changes: 8 additions & 7 deletions src/status_im/contexts/wallet/add_account/create_account/events.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
:on-success on-success
:on-error (fn [error]
(log/error
"Failed to resolve next path derivation path"
"Failed to resolve next derivation path"
{:event :wallet/next-derivation-path
:method "accounts_resolveSuggestedPathForKeypair"
:error error
Expand Down Expand Up @@ -191,17 +191,18 @@

(rf/reg-event-fx
:wallet/derive-address-and-add-account
(fn [_ [{:keys [password derived-from-address derivation-path account-preferences]}]]
(fn [_ [{:keys [password derived-from-address derivation-path account-preferences key-uid]}]]
{:fx [[:json-rpc/call
[{:method "wallet_getDerivedAddresses"
:params [(security/safe-unmask-data password)
derived-from-address
[derivation-path]]
:on-success (fn [[derived-account]]
(rf/dispatch [:wallet/add-account
(assoc account-preferences :password password)
(assoc account-preferences
:key-uid key-uid
:password password)
derived-account]))
:on-error #(log/info "Failed to get derived addresses"
derived-from-address
%)}]]]}))

:on-error [:wallet/log-rpc-error
{:event :wallet/derive-address-and-add-account
:params [derived-from-address [derivation-path]]}]}]]]}))
3 changes: 1 addition & 2 deletions src/status_im/contexts/wallet/add_account/create_account/select_keypair/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,7 @@
[quo/bottom-actions
{:actions :one-action
:button-one-label (i18n/label :t/confirm-account-origin)
:button-one-props {:disabled? (= selected-keypair selected-key-uid)
Copy link
Member

Choose a reason for hiding this comment

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

👍

:customization-color customization-color
:button-one-props {:customization-color customization-color
:on-press #(rf/dispatch [:wallet/confirm-account-origin
selected-key-uid])}
:container-style style/bottom-action-container}]]))
4 changes: 2 additions & 2 deletions src/status_im/contexts/wallet/add_account/create_account/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
:action-props {:on-press #(rf/dispatch [:standard-auth/authorize
{:on-auth-success on-auth-success
:auth-button-label (i18n/label :t/continue)}])

:button-text (i18n/label :t/edit)
:icon-left :i/face-id
:alignment :flex-start}
Expand Down Expand Up @@ -217,6 +216,7 @@
[:wallet/derive-address-and-add-account
{:password password
:derived-from-address derived-from
:key-uid key-uid
:derivation-path @derivation-path
:account-preferences preferences}])))
[derived-from])]
Copy link
Member

Choose a reason for hiding this comment

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

Should we add key-uid to the deps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll add it 👍

Expand All @@ -231,7 +231,7 @@
{:account-color @account-color
:slide-button-props {:on-auth-success on-auth-success
:disabled? (or (empty? @account-name)
(= "" @derivation-path)
(string/blank? @derivation-path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just making the code more robust

(some? error))}}
[avatar
{:account-color @account-color
Expand Down
7 changes: 3 additions & 4 deletions src/status_im/contexts/wallet/events.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,12 @@
[:dispatch [:wallet/clear-create-account]]]}))

(rf/reg-event-fx :wallet/add-account
(fn [{:keys [db]}
[{:keys [password account-name emoji color type]
(fn [_
[{:keys [key-uid password account-name emoji color type]
:or {type :generated}}
{:keys [public-key address path] :as _derived-account}]]
(let [lowercase-address (some-> address
(string/lower-case))
key-uid (get-in db [:wallet :ui :create-account :selected-keypair-uid])
string/lower-case)
account-config {:key-uid (when (= type :generated) key-uid)
:wallet false
:chat false
Expand Down