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

Bug/improved wallet styles #2053

Merged
merged 1 commit into from
Oct 11, 2017
Merged

Bug/improved wallet styles #2053

merged 1 commit into from
Oct 11, 2017

Conversation

jeluard
Copy link
Contributor

@jeluard jeluard commented Oct 9, 2017

Summary:

  • improved wallet name style
  • do not use default amount for wallet detail
  • fixed broken button style

Steps to test:

  • Open Status
  • Navigate wallet tab
  • Verify long wallet name are shorten in the middle
  • Verify wallet details amount is correct
  • Verify wallet button are on a single line

status: ready

:letter-spacing 0.2}
{:font-size 14
:padding-vertical 10
:letter-spacing 0.5}
Copy link
Contributor

Choose a reason for hiding this comment

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

last time I checked letter-spacing doesn't work with react-native on Android

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, see #2045. But it might at some point so better add it now.

@@ -24,12 +24,9 @@
(let [amount' (string/replace amount #"," ".")
amount-splited (string/split amount' #"[.]")]
(cond
(or (nil? amount) (= amount "") (re-matches #"0[,.]0*$" amount))
(or (nil? amount) (= amount "") (= amount "0") (re-matches #"0[,.]0*$" amount))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we name whatever this condition is? Looks like something like no-amount-present?, could be in let or if we re-use it.

Not part of PR, but I also noticed there is some weird parse float / try-catch with money just under this piece of code, it seems like that belongs in money.cljs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

(ns status-im.ui.screens.wallet.wallet-list.subs
(:require [re-frame.core :as re-frame]))

;; TODO update when adding multiple wallet support. This will probably require changes to app-db
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO(name)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -36,16 +36,17 @@
(border position)))

(defnstyle button-text [disabled?]
{:font-size 15
:font-weight "normal"
{:font-weight "normal"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be :normal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@yenda yenda added the wallet label Oct 10, 2017
@yenda yenda added this to the 0.9.12 milestone Oct 10, 2017
@jeluard jeluard force-pushed the bug/improved-wallet-styles branch from 964b32c to efbc32c Compare October 10, 2017 10:52
@jeluard jeluard self-assigned this Oct 10, 2017
@annadanchenko annadanchenko self-assigned this Oct 10, 2017
@rasom
Copy link
Contributor

rasom commented Oct 11, 2017

@jeluard please resolve conflicts

@annadanchenko
Copy link

Agreed to change the long wallet title. It will be like 'Основной кош..' instead of Основн…ошелек

@jeluard jeluard force-pushed the bug/improved-wallet-styles branch from efbc32c to 80f9f8c Compare October 11, 2017 10:02
Make wallet list show correct amount
Wallet name should be cut if too long
@jeluard jeluard force-pushed the bug/improved-wallet-styles branch from 80f9f8c to a189163 Compare October 11, 2017 10:05
@rasom rasom merged commit fb10b58 into develop Oct 11, 2017
@rasom rasom deleted the bug/improved-wallet-styles branch October 11, 2017 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants