-
-
Notifications
You must be signed in to change notification settings - Fork 43
Conversation
I like the screenshots.
I actually like the current layout. It matches the structure of a TUM ID.
I don't know about the TUM tower. But from user feedback we know that the token activation process could use some illustration. |
You can find the TUM tower illustration on this site: Link. It probably only makes sense to add this illustration if we switched from the three textfields to a single button, as the immediately appearing keyboard would otherwise cover it. This is what it would look like: I’ll have a look at how we can support the token activation process with an illustration. |
@thellmund nice! Could we also in one go make |
It already is |
@TG908 cool beans - also with the same naming? "Connect to..." In Android don't show the login screen at all, but only a card on the first launch and have a menu entry for it. |
Its right under the login button. It says continue without tum Id |
…tify-onboarding
…tify-onboarding
) | ||
alert.addAction( | ||
UIAlertAction(title: "Continue", style: .default) { action in | ||
guard let text = alert.textFields![0].text else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to force unwrap here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch 👍 I changed it to a chain of optionals.
|
||
// ************* | ||
// | ||
// TODO: Make sure login for App Store review still works! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TG908 This branch is still a work in progress, as I’ll have to adjust the logic to accommodate the App Store reviewers, which don’t have a TUM ID. You can revoke your approval again 😄 |
@thellmund I think they do have one. Checkout the App Review Information in our latest submission |
@TG908 Technically yes, but it does not conform to the usual pattern (ga12abc) and is not 7 characters long. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix login issue for app review
SonarQube analysis reported 2 issues Watch the comments in this conversation to review them. 1 extra issueNote: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
@TG908 I actually like the Popup - it makes the signup process to be super simple and cleaner. Can we bring this branch to a mergeable state? |
closed due to inactivity |
@TG908 till will probably not contribute too much to iOS in the future. I guess you don't want to change the onboarding? |
@kordianbruck onboarding is not my first priority. |
@TG908 alright, then just lets keep the branch around, in case somebody wants to pick this up in the future. |
The current onboarding screens have the following problems:
Here’s a potential update to the onboarding screens:
Let me know what you think.
Open questions: