Esri / arcgis-maps-sdk-swift-toolkit

Mapping components that will simplify your Swift app development with the ArcGIS Maps SDK for Swift.
https://developers.arcgis.com/swift
Apache License 2.0
29 stars 7 forks source link

Edit UI for Authenticator #878

Closed zkline101 closed 1 week ago

zkline101 commented 2 weeks ago

cc: @rolson @philium

Swift #6015

These edits were originally meant to give CredentialInputSheetView a more native feel on visionOS but then I realized these edits can also apply to iOS and Mac Catalyst. These edits make the credential sheet look more like the credential sheet in the Mail app. I explain more of my changes in the comments below.

visionOS before changes: CredentialInput_before

visionOS after changes: After

Apple's credential form in Mail app: Credential_Apple

iOS after changes: iPhone

zkline101 commented 2 weeks ago

Please wait to review, I see a few more edits I want to make.

zkline101 commented 2 weeks ago

@njarecha @dfeinzimer This is now ready for review, thank you.

njarecha commented 2 weeks ago

@zkline101 Please revert the changes of using alerts. There are issues with it. Please see PR where we made the changes from alert to sheet.

zkline101 commented 2 weeks ago

Dang, alright. The only changes I have then for the trust host prompt and certificate picker is to have the buttons on the top of the sheet.

zkline101 commented 2 weeks ago

Thanks @njarecha I reverted the alert changes, this is open for review.

njarecha commented 1 week ago

@zkline101 If we are making changes to the credential input view then I would like to see it like this,

Simulator Screenshot - iPhone 16 Pro Max - 2024-09-23 at 09 20 21

instead of this, Simulator Screenshot - iPhone 16 Pro Max - 2024-09-23 at 09 20 55

rolson commented 1 week ago

@zkline101 If we are making changes to the credential input view then I would like to see it like this,

I would prefer to hold off on making UI edits until we have suggestions from the UI designer who is starting soon.

zkline101 commented 1 week ago

@dfeinzimer @njarecha @rolson Ok I moved the buttons back to the original location but kept the Form. This is ready for review.

iOS: Simulator Screenshot - iPhone 16 Pro Max - 2024-09-23 at 12 55 28

visionOS: Simulator Screenshot - AVP Model 2 - 2024-09-23 at 12 53 49

zkline101 commented 1 week ago

Just one suggestion. Also, noticed that GeometryReader { proxy in is not used anywhere. Shall we remove it?

Yup looks like that can go. It wasn't used in the original implementation either.

zkline101 commented 1 week ago

Thanks everyone for the feedback.