Use Monospace font for Address Labels #127
Conversation
borwahs
commented
Mar 30, 2019
borwahs
commented
- Set font for Address UILabels to monospace (Menlo).
There was a problem hiding this comment.
I noticed you're using Menlo here. There are other various places in the code that use monospacedDigitSystemFont, which I think doesn't look as good as Menlo. We should standardize on one or the other, so can you please replace those with Menlo as well?
A clean way to do this that would allow us to easily change the font in the future would be to add an extension file called UIFont+Monospaced.swift in the extensions folder, and add a class func balanceMonospacedDigitSystemFont(ofSize fontSize: CGFloat, weight: UIFont.Weight) -> UIFont function so we only have to change the font in one place.
| @@ -14,9 +14,10 @@ class WalletTableViewCell: UITableViewCell { | |||
|
|
|||
| let addressLabel: UILabel = { | |||
| let label = UILabel() | |||
| label.font = UIFont.systemFont(ofSize: 12) | |||
| label.font = UIFont.init(name: "Menlo", size: 12.0) | |||
There was a problem hiding this comment.
the .init can be removed
Balance/Views/WalletInfoView.swift
Outdated
| @@ -35,7 +35,7 @@ class WalletInfoView: UIView { | |||
|
|
|||
| private let addressLabel: UILabel = { | |||
| let addressLabel = UILabel() | |||
| addressLabel.font = UIFont.monospacedDigitSystemFont(ofSize: 12, weight: .regular) | |||
| addressLabel.font = UIFont.init(name: "Menlo", size: 12.0) | |||
There was a problem hiding this comment.
the .init can be removed
|
On it. Was debating whether to build an extension and change the existing ones. |
|
See comparison of pre and post Menlo usage below. Couple of notes:
|
|
Thoughts on changing the CDP Modal Info view numbers to the monospace font? |
|
Totally agree @borwahs. There are lots of places where I want to use monospaced fonts and this really helps me see how to do it. Thanks! I'll let Ben review this before merging. |
|
Really interesting discussion here: https://stackoverflow.com/questions/38148073/uifont-monospaceddigitsystemfontofsize-not-really-monospaced In all the designs I use I think we need to find a monospaced font that works well for the project. I tweeted about this before. Will look at the suggestions. |
|
@ricburton // nice find on that SO question. I was confused why monospaced system don’t was not actually using it. Looks like the adjust font to size of container option breaks it. |
|
Yeah it’s really annoying they don’t make it available... we snuck it into balance for Mac and they approved it but they could reject at any time so it’s better to find a different one that’s fully allowed |
|
Will hold off on merging this until we settle on the font we want. |
|
Also for the name of the extension swift file, I accidentally wrote |
borwahs
force-pushed
the
set-address-font-to-monospace
branch
from
7295798 to
c133646
Compare
|
I did something odd here. I think I might have not branched off of develop. Not sure why there are so many conflicts. Going to look into it and repush. |
borwahs
force-pushed
the
set-address-font-to-monospace
branch
from
c133646 to
7d0f64e
Compare
borwahs
force-pushed
the
set-address-font-to-monospace
branch
from
7d0f64e to
1db3eac
Compare
|
fixed merge issues. think i accidentally rebased master at some point. |
|
@ricburton are you happy with Menlo as the font? |
