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

[tvOS] Login Flow Cleanup - Second Pass #1403

Merged
merged 14 commits into from
Jan 24, 2025
Merged

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Jan 21, 2025

Login Flow UI Updates

Initial Login Screen

Connect to Server - Remove Duplicate Icons

New Version:
New Initial Screen

Previous Version:
Old Initial Screen

Server Management

Server Bar - Ensure Menus are same sizing

New Version:
New Server Bar

Previous Version:
Old Server Bar

Server Configuration

Edit Server View - Clean up menu buttons

New Edit Server View

Additional UI Elements

User Sign-In - Add Background Image for Server

Login View Background


Outstanding Issue

tvOS User Sign-In Crash

This could be an emulator issue but I see this issue where the first time I login with a new user, Swiftfin crashes. Restarting, the user still exists but it's not great to restart after each login.

Screenshot 2025-01-21 at 07 52 19

Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

As I was typing a comment for this change I thought instead to just implement the same Menu/Button conditioning for server selection when it is all.

Using the user selection a bit more, I think we may want to move the Add User button from an inline item with the users and instead have it be a button on the left/leading side of the server selection to make the stack symmetrical. Maybe also for iOS to move it to the top trailing menu. We can take a look at that later.

@JPKribs
Copy link
Member Author

JPKribs commented Jan 21, 2025

Using the user selection a bit more, I think we may want to move the Add User button from an inline item with the users and instead have it be a button on the left/leading side of the server selection to make the stack symmetrical. Maybe also for iOS to move it to the top trailing menu. We can take a look at that later.

I can do that! One item of note, on user sign in on tvOS, disabled buttons square off. IMO, it's a little wonky looking but not an issue per-say. Wanted to run it by you while I was touching up login flow:
Simulator Screenshot - Apple TV 4K (3rd generation) - 2025-01-21 at 12 15 52

I believe I have this resolved with the rounding:
Simulator Screenshot - Apple TV 4K (3rd generation) - 2025-01-21 at 15 04 37

ListRowButtons should now better reflect iOS as well.

@JPKribs

This comment was marked as outdated.

@JPKribs
Copy link
Member Author

JPKribs commented Jan 21, 2025

Okay! Take 2. I've made some additional changes.

  • I updated the ListRowButton to have a .destructive that mirrors iOS.
  • I updated the ListRowButton to retain its rounded corners even when disabled.
  • Delete button now uses the same ListRowButton .destructive to mirror.
Delete User Buttons

Selected User - Non-Focused Delete:
Selected User   Non-Highlighted Delete

Selected User - Focused Delete:
Selected User   Highlighted Delete

Disabled Delete:
Disabled Delete

Server Details Page Buttons Old Buttons were cards inside of forms... I actually did this a while ago so that's on me: Screenshot 2025-01-21 at 15 03 16

The new buttons fill this out appropriately:
Screenshot 2025-01-21 at 15 05 46

Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

Let's leave out the add user button changes as that change would also consist of more cleanup within SelectUserView.

@JPKribs
Copy link
Member Author

JPKribs commented Jan 23, 2025

Let's leave out the add user button changes as that change would also consist of more cleanup within SelectUserView.

Sounds good. Those changes should now be reverted!

Edit: Added a label while I am in these files. Also, updated the screenshots for this PR to reflect it's current version.

JPKribs and others added 4 commits January 23, 2025 20:05
Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

Looks great!

@LePips LePips merged commit b0b604c into jellyfin:main Jan 24, 2025
4 checks passed
@JPKribs JPKribs deleted the tvOSLoginCleanup branch January 24, 2025 06:02
@JPKribs JPKribs added the enhancement New feature or request label Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants