BlockchainCommons / GordianWallet-iOS

iOS wallet linked by Torgap to your own full-node server
Other
47 stars 14 forks source link

LogInViewController update #13

Closed brett-doffing closed 4 years ago

brett-doffing commented 4 years ago

Moved UIViewController extension to Utilities. Instantiated LogInViewController directly in SceneDelegate. Setup use of auto layout in LogInViewController. Moved additional protocols to an extension for separation.

brett-doffing commented 4 years ago

More looking to see if there is anything else to do or add here, while simply giving an example of how I would implement this.

ChristopherA commented 4 years ago

This PR is still marked as a "work in progress". Let me know when it is ready for review after @Fonta1n3 builds and tests it by having either @doffing81 or @Fonta1n3 click on "ready for review" button.

Fonta1n3 commented 4 years ago

@doffing81 Please let me know if you want me to merge this or if it is still a WIP?

brett-doffing commented 4 years ago

I was looking to see if anything else should be done. If there isn't anything more, then it should be ok to merge.

Fonta1n3 commented 4 years ago

There are conflicts so looks like you need to rebase before it can be merged, my previous PR had made some changes to the LogInViewController which looks to be the culprit.

Fonta1n3 commented 4 years ago

@doffing81 Just FYI I will shoot you an email with regards to to do items, there are many ;) Thank you!

Fonta1n3 commented 4 years ago

NIT:

Can we keep the existing frame logInButton.frame = CGRect(x: 32, y: 100, width: view.frame.width - 64, height: 80)

and the existing text styling: description.font = .systemFont(ofSize: 16, weight: .light)

and the existing text description: description.text = "Blockchain Commons, LLC and FullyNoded 2 do not use or save your Apple ID data in anyway, it is used solely for 2FA (two-factor authentication) purposes only."

brett-doffing commented 4 years ago

Keep the existing frame as in not use constraints? I can definitely keep/change whatever sizing, styling, and position. This then begs what I should do with the sizing and positioning of the label, other than the text itself?

You don't find it weird to say that you don't use the Apple ID, while using it, and then saying that you use it?

Fonta1n3 commented 4 years ago

You can programmatically use the data, we do not. Please keep all existing frames as they are.

Fonta1n3 commented 4 years ago

To clarify when I built the PR the button moved significantly down the screen. I guess I should say keep the existing position, please.

brett-doffing commented 4 years ago

I am just going to close this PR as it was originally meant to show a different way of doing things, while possibly being added to, but it doesn’t look like anything is to be added, the methodology has been shown, and everything is to be kept as it was before the PR, so it is nothing more than redundant.