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

Login Screen Work #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

dexorc
Copy link
Contributor

@dexorc dexorc commented Jan 20, 2020

This PR is to make the login screen more interactable.

  • Dismiss the keyboard or be able to hop to "Next" field by tapping off the input field
  • Make it so when the user has put in required values, the screen can then transition to the next selection

Still need to save player info state, but keeping this small to get this screen moving.

}

if(notification.name == LoginViewController.userNameNotification) {
self.userName = notification.userInfo!["userName"] as! String
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this map returns null when you ask for a String key, then I'd suggest combining these notifications into a single notification and checking all of the possible keys any time something changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In Kotlin, we can do a thing like:

(notification.userInfo!["userName"] as? String)?.let { userName = it }

This basically:

  • queries the map for the specified key, then attempts to convert that to a string
  • if the end result of that is not null, then enter the block, setting an implicit variable it to the converted string
  • the block sets the local userName variable

self.twitterName = notification.userInfo!["twitterName"] as! String
}

// Remove when comfortable with functionality
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Remove when comfortable with functionality
// FIXME Remove when comfortable with functionality

This makes finding debug-only code more searchable.


if(notification.name == LoginViewController.userNameNotification) {
self.userName = notification.userInfo!["userName"] as! String
if (self.userName != "") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (self.userName != "") {
signInButton.isEnabled = self.userName != ""

if(notification.name == LoginViewController.userNameNotification) {
self.userName = notification.userInfo!["userName"] as! String
if (self.userName != "") {
signInButton.isEnabled = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
signInButton.isEnabled = true

self.userName = notification.userInfo!["userName"] as! String
if (self.userName != "") {
signInButton.isEnabled = true
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
} else {

if (self.userName != "") {
signInButton.isEnabled = true
} else {
signInButton.isEnabled = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
signInButton.isEnabled = false

signInButton.isEnabled = true
} else {
signInButton.isEnabled = false
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
}

God this feature sucks.

@@ -11,16 +11,66 @@
import UIKit

class LoginViewController: UIViewController {

static let userNameNotification = Notification.Name("userNameNotification")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We really just need one of these. Call it loginInfoNotification

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.

2 participants