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

Jump to bottom

No ability to enter/paste assets exceeds the users's balance #18526 #18599

Merged
merged 9 commits into from
Jan 24, 2024
Merged

No ability to enter/paste assets exceeds the users's balance #18526 #18599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a911fc5
change functionality
mmilad75 Jan 23, 2024
58889ad
fix handle-swap
mmilad75 Jan 23, 2024
cd7832e
fix lint issues
mmilad75 Jan 23, 2024
3a2ad0d
update tests
mmilad75 Jan 23, 2024
b81e265
Merge branch 'develop' into milad/18526-update-send-assets-error-ui
mmilad75 Jan 24, 2024
ffe2400
change error to error?
mmilad75 Jan 24, 2024
12a975b
change reset-input-error
mmilad75 Jan 24, 2024
b01cb21
update tests
mmilad75 Jan 24, 2024
cc23a08
fix lint issues
mmilad75 Jan 24, 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
8 changes: 6 additions & 2 deletions src/quo/components/wallet/token_input/style.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@
:height "100%")))

(defn text-input
[theme]
(assoc text-input-dimensions :color (colors/theme-colors colors/neutral-100 colors/white theme)))
[theme error?]
(assoc text-input-dimensions
:color
(if error?
(colors/resolve-color :danger theme)
(colors/theme-colors colors/neutral-100 colors/white theme))))

(defn placeholder-text
[theme]
Expand Down
11 changes: 7 additions & 4 deletions src/quo/components/wallet/token_input/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@
(crypto-format num-value conversion crypto-decimals token))))

(defn- data-info
[{:keys [theme token crypto-decimals conversion networks title crypto? currency amount]}]
[{:keys [theme token crypto-decimals conversion networks title crypto? currency amount error?]}]
[rn/view {:style style/data-container}
[network-tag/view {:networks networks :title title}]
[network-tag/view
{:networks networks
:title title
:status (when error? :error)}]
[text/text
{:size :paragraph-2
:weight :medium
Expand Down Expand Up @@ -80,7 +83,7 @@
(reset! value-atom v))
(when on-change-text
(on-change-text v)))]
(fn [{:keys [theme token customization-color show-keyboard? crypto? currency value]
(fn [{:keys [theme token customization-color show-keyboard? crypto? currency value error?]
:or {show-keyboard? true}}]
[rn/pressable
{:on-press focus-input
Expand All @@ -90,7 +93,7 @@
:size :size-32}]
[rn/view {:style style/text-input-container}
[rn/text-input
(cond-> {:style (style/text-input theme)
(cond-> {:style (style/text-input theme error?)
:placeholder-text-color (style/placeholder-text theme)
:auto-focus true
:ref set-ref
Expand Down
4 changes: 3 additions & 1 deletion src/status_im/contexts/preview/quo/wallet/token_input.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
{:key :currency
:type :select
:options [{:key :usd}
{:key :eur}]}])
{:key :eur}]}
{:key :error?
:type :boolean}])

(defn view
[]
Expand Down
52 changes: 13 additions & 39 deletions src/status_im/contexts/wallet/send/input_amount/component_spec.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
(h/is-truthy (h/get-by-text "0"))
(h/is-truthy (h/get-by-text "ETH"))
(h/is-truthy (h/get-by-text "$0.00"))
(h/is-truthy (h/get-by-label-text :container))
(h/is-disabled (h/get-by-label-text :button-one)))

(h/test "Fill token input and confirm"
Expand All @@ -77,6 +78,7 @@
(-> (h/wait-for #(h/get-by-text "$1234.50"))
(.then (fn []
(h/is-truthy (h/get-by-label-text :button-one))
(h/is-truthy (h/get-by-label-text :container))
(h/fire-event :press (h/get-by-label-text :button-one))
(h/was-called on-confirm))))))

Expand All @@ -99,61 +101,33 @@
(-> (h/wait-for #(h/get-by-text "$1234.50"))
(.then (fn []
(h/is-truthy (h/get-by-label-text :button-one))
(h/is-truthy (h/get-by-label-text :container))
(h/fire-event :press (h/get-by-label-text :button-one))
(h/was-called on-confirm))))))
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we deleting all these tests? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the tests were covering the input of more than limit which were reset to the limit. Now, when entering more than limit, nothing should happen except for changing the style.


(h/test "Try to fill more than limit"
(h/setup-subs sub-mocks)
(h/render [input-amount/view
{:crypto-decimals 10
:limit-crypto 286}])
{:crypto-decimals 1
:limit-crypto 1}])

(h/fire-event :press (h/query-by-label-text :keyboard-key-2))
(h/fire-event :press (h/query-by-label-text :keyboard-key-9))
(h/fire-event :press (h/query-by-label-text :keyboard-key-5))

(-> (h/wait-for #(h/is-truthy (h/get-by-text "$290.00")))
(.then (fn []
(h/fire-event :press (h/query-by-label-text :keyboard-key-backspace))
(h/fire-event :press (h/query-by-label-text :keyboard-key-8))
(h/fire-event :press (h/query-by-label-text :keyboard-key-5))
(h/wait-for #(h/get-by-text "$2850.00"))))))
(h/is-truthy (h/get-by-label-text :container-error)))

(h/test "Try to fill more than limit"
Copy link
Contributor

Choose a reason for hiding this comment

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

this test should still work but the check should be updated 👍 .
we can technically fill more than limit no? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same thing here

(h/test "Switch from crypto to fiat and check limit"
(h/setup-subs sub-mocks)
(h/render [input-amount/view
{:crypto-decimals 10
:limit-crypto 286
{:crypto-decimals 1
:limit-crypto 1
:on-confirm #()}])

(h/fire-event :press (h/query-by-label-text :keyboard-key-2))
(h/fire-event :press (h/query-by-label-text :keyboard-key-9))
(h/fire-event :press (h/query-by-label-text :keyboard-key-5))

(-> (h/wait-for #(h/get-by-text "$290.00"))
(.then (fn []
(h/fire-event :press (h/query-by-label-text :keyboard-key-backspace))
(h/fire-event :press (h/query-by-label-text :keyboard-key-8))
(h/fire-event :press (h/query-by-label-text :keyboard-key-5))
(h/wait-for #(h/get-by-text "$2850.00"))))))

(h/test "Switch from crypto to fiat and check limit"
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems like a valid test case still

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, when the input was more than the limit and a switch was happening, the input was changing to the limit. Now, nothing should happen except for the style changing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

(h/setup-subs sub-mocks)
(h/render [input-amount/view
{:crypto-decimals 2
:limit-crypto 250
:on-confirm #()}])
(h/is-truthy (h/get-by-label-text :container-error))
(h/fire-event :press (h/query-by-label-text :reorder))

(h/fire-event :press (h/query-by-label-text :keyboard-key-2))
(h/fire-event :press (h/query-by-label-text :keyboard-key-0))
(-> (h/wait-for #(h/get-by-text "$200.00"))
(.then (fn []
(h/fire-event :press (h/query-by-label-text :reorder))
(h/wait-for #(h/get-by-text "2.00 ETH"))))
(.then (fn []
(h/fire-event :press (h/query-by-label-text :keyboard-key-5))
(h/fire-event :press (h/query-by-label-text :keyboard-key-5))
(h/wait-for #(h/get-by-text "20.50 ETH"))))
(-> (h/wait-for #(h/get-by-text "Max: 1000.00 USD"))
(.then (fn []
(h/fire-event :press (h/query-by-label-text :keyboard-key-5))
(h/wait-for #(h/get-by-text "20.50 ETH")))))))
(h/wait-for #(h/is-truthy (h/get-by-label-text :container))))))))
37 changes: 24 additions & 13 deletions src/status_im/contexts/wallet/send/input_amount/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@
(>= (js/parseFloat balance) input-value)))
(map first)))

(defn- reset-input-error
[new-value prev-value input-error]
(reset! input-error
(> new-value prev-value)))

(defn- f-view-internal
;; crypto-decimals and limit-crypto args are needed for component tests only
[{:keys [crypto-decimals limit-crypto]}]
Expand All @@ -75,6 +80,7 @@
limit-fiat (.toFixed (* (:total-balance token) conversion-rate) 2)
crypto-decimals (or crypto-decimals (utils/get-crypto-decimals-count token))
input-value (reagent/atom "")
input-error (reagent/atom false)
current-limit (reagent/atom {:amount limit-crypto
:currency token-symbol})
handle-swap (fn [crypto?]
Expand All @@ -84,27 +90,30 @@
:currency token-symbol}
{:amount limit-fiat
:currency currency}))
(when (> num-value (:amount @current-limit))
(reset! input-value ""))))
(reset-input-error num-value
(:amount @current-limit)
input-error)))
handle-keyboard-press (fn [v]
(let [current-value @input-value
new-value (make-new-input current-value v)
num-value (or (parse-double new-value) 0)]
(when (and (not loading-suggested-routes?)
(<= num-value (:amount @current-limit)))
(let [current-value @input-value
new-value (make-new-input current-value v)
num-value (or (parse-double new-value) 0)
current-limit-amount (:amount @current-limit)]
(when (not loading-suggested-routes?)
(reset! input-value new-value)
(reset-input-error num-value current-limit-amount input-error)
(reagent/flush))))
handle-delete (fn [_]
(when-not loading-suggested-routes?
(swap! input-value #(subs % 0 (dec (count %))))
(reagent/flush)))
(let [current-limit-amount (:amount @current-limit)]
(swap! input-value #(subs % 0 (dec (count %))))
(reset-input-error @input-value current-limit-amount input-error)
(reagent/flush))))
handle-on-change (fn [v]
(when (valid-input? @input-value v)
(let [num-value (or (parse-double v) 0)
current-limit-amount (:amount @current-limit)]
(if (> num-value current-limit-amount)
(reset! input-value (str current-limit-amount))
(reset! input-value v))
(reset! input-value v)
(reset-input-error num-value current-limit-amount input-error)
(reagent/flush))))]
(fn [{:keys [on-confirm]
:or {on-confirm #(rf/dispatch [:wallet/send-select-amount
Expand Down Expand Up @@ -135,7 +144,8 @@
100)))
[@input-value])
[rn/view
{:style style/screen}
{:style style/screen
:accessibility-label (str "container" (when @input-error "-error"))}
[account-switcher/view
{:icon-name :i/arrow-left
:on-press #(rf/dispatch [:navigate-back-within-stack :wallet-send-input-amount])
Expand All @@ -145,6 +155,7 @@
:token token-symbol
:currency currency
:crypto-decimals crypto-decimals
:error? @input-error
:networks (:networks token)
:title (i18n/label :t/send-limit {:limit limit-label})
:conversion conversion-rate
Expand Down