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

Improve UX when the user is not logged in #330

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PWrzesinski
Copy link

@PWrzesinski PWrzesinski commented Dec 3, 2022

When running XcodesApp for the first time, it took me a while to figure out that I need to log in before I can download Xcode - it was a while ago, but the error was quite unclear at the time (see #164 and #165). This has been fixed since, but I hope making the button red when the user is not authenticated will improve the UX a little bit, guiding the user to click it.

It's a minor improvement that was at the back of my mind ever since that first time and I finally got around to implementing it. Please let me know if this kind of UI change is welcome or not.

Xcodes_red_auth_button

@PWrzesinski PWrzesinski requested a review from a team as a code owner December 3, 2022 15:29
@MattKiazyk
Copy link
Contributor

Hey @PWrzesinski Thanks for taking the time for this PR!

Couple of thoughts:

  • I agree that there should be something to notify the user they need to log in, I don't mind the red, but I'm almost wondering if words should also appear on next to the button - similar to how filters work:

image

- Secondly - that `authenticationState` needs some work as by default when you first open the app, it doesn't even check it, so it will always be red when you first open up the app, and only show what it actually is after it's refreshed. So I think there's some improvement there that needs to happen at the same time for the red icon on the main list to be useful.

Let me know if you have any questions.

@PWrzesinski
Copy link
Author

PWrzesinski commented Dec 7, 2022

Thanks for taking a look @MattKiazyk! I've updated the implementation. Here's how it looks currently:

  • At the start:

Xcodes_auth_spinner

  • When not logged in:

Xcodes_red_auth_button_2

Considering if the red color is needed now, or perhaps the default one will do nice. I've added the check for authentication state in the AppState init, is that ok? Completely new to this code base.

I couldn't fully test the new functionality as it seems I'm logged out every time I start the app, probably denied it access to the keychain or something.

Also got a couple of general questions:

  • How do I go about localizing the new copy?
  • In Contributing.md there is a mention of linting, but I can't find any configuration in the project - ok to ignore, or should I just run Swiftlint with a default config?
  • Didn't add any tests for the changes, doesn't feel necessary here.

@PWrzesinski PWrzesinski changed the title Make the authentication button red when the user is not logged in Improve UX when the user is not logged in Jan 5, 2023
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