Design for seed backup and move new wallet seed verification to after setup #502
Conversation
…ation to after setup
| @@ -29,7 +29,7 @@ | |||
| <rect key="frame" x="0.0" y="0.0" width="414" height="896"/> | |||
| <autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/> | |||
| <subviews> | |||
| <stackView hidden="YES" opaque="NO" contentMode="scaleToFill" axis="vertical" alignment="center" spacing="6" translatesAutoresizingMaskIntoConstraints="NO" id="EJZ-78-Rgs"> | |||
| <stackView hidden="YES" opaque="NO" contentMode="scaleToFill" axis="vertical" alignment="center" spacing="8" translatesAutoresizingMaskIntoConstraints="NO" id="EJZ-78-Rgs"> | |||
There was a problem hiding this comment.
Why is this spacing increased to 8? Have you considered how devices with shorter screen height will be affected by this change? I don't see this change as being necessary to the feature being added by this PR. Revert please.
| <view hidden="YES" clipsSubviews="YES" contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="aaU-uk-JKT"> | ||
| <rect key="frame" x="8" y="110.66666666666666" width="398" height="48"/> | ||
| <subviews> | ||
| <stackView opaque="NO" contentMode="scaleToFill" distribution="fillProportionally" alignment="center" translatesAutoresizingMaskIntoConstraints="NO" id="3zw-6M-uXq"> |
There was a problem hiding this comment.
-
This
StackViewis the only child of the parentView. Tells me that the parent view element is unnecessary, why not use theStackViewdirectly rather than embed it inside aView? -
Also, this
StackViewholds a text and an image. Can't a button be used for this? -
Summary, replace this
viewandstackviewwith abutton
| @@ -15,6 +15,8 @@ class OverviewViewController: UIViewController { | |||
| @IBOutlet weak var fetchingBalanceIndicator: UIImageView! | |||
| @IBOutlet weak var totalBalanceLabel: UILabel! | |||
| @IBOutlet weak var recentActivityTableView: UITableView! | |||
| @IBOutlet weak var backupWallet: UIView! | |||
| @IBOutlet weak var backupViewSpacing: UIView! | |||
There was a problem hiding this comment.
- I don't really see a need for this
IBOutlet - The view referenced by this
IBOutletwas added before this PR and it's purpose was certainly not to space the newly added backup view. This tells me that this variable is named wrongly. Which further leads me to question its usage.
| let tapGesture = UITapGestureRecognizer(target: self, action: #selector(backupSeedAction)) | ||
| self.backupWallet.addGestureRecognizer(tapGesture) | ||
| self.backupWallet.isHidden = false | ||
| self.backupViewSpacing.isHidden = true |
There was a problem hiding this comment.
As alluded to in the preceding comment, I don't see why it is necessary to hide this spacing.
|
This issue is moved to version 2 UI #530 |
closes #461