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

feat: authentication flow views - On premise - WPB-15225 #2531

Open
wants to merge 80 commits into
base: develop
Choose a base branch
from

Conversation

KaterinaWire
Copy link
Contributor

@KaterinaWire KaterinaWire commented Feb 12, 2025

TaskWPB-15225 [iOS] Implement new authentication flow views - Login - On premise, claimed, no sso

Issue

Implement new authentication flow views -> Login -> On premise:
Screenshot 2025-02-13 at 16 13 34

Screenshot 2025-02-13 at 16 13 41

Testing

Checklist

  • Title contains a reference JIRA issue number like [WPB-XXX].
  • Description is filled and free of optional paragraphs.
  • Adds/updates automated tests.

UI accessibility checklist

If your PR includes UI changes, please utilize this checklist:

  • Make sure you use the API for UI elements that support large fonts.
  • All colors are taken from WireDesign.ColorTheme or constructed using WireDesign.BaseColorPalette.
  • New UI elements have Accessibility strings for VoiceOver.

KaterinaWire and others added 30 commits January 28, 2025 09:22
@KaterinaWire KaterinaWire marked this pull request as ready for review February 13, 2025 15:58
Copy link
Contributor

github-actions bot commented Feb 13, 2025

Test Results

32 tests   32 ✅  9s ⏱️
 3 suites   0 💤
 1 files     0 ❌

Results for commit 504cbb4.

♻️ This comment has been updated with latest results.


import Foundation

public struct BackendEnvironment {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@johnxnguyen I'm not sure where we should define BackendEnvironment, since we have it in WireAPI too

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should use the one defined in WireAPI rather than duplicating it. Did you try it?

}, label: {
Text(L10n.OnPremUserLogin.title(viewModel.backendName) + " ")
.foregroundColor(ColorTheme.Buttons.Secondary.onEnabled.color)
+ Text(Image(systemName: "info.circle"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm struggling to fix this SwiftFormat issue

@State private var showPasswordRules: Bool = false
@State private var showCustomBackendAlert = false

private var proxyEmail: String = ""
Copy link
Collaborator

@johnxnguyen johnxnguyen Feb 14, 2025

Choose a reason for hiding this comment

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

question: shouldn't this also be @State?

Copy link
Collaborator

@johnxnguyen johnxnguyen left a comment

Choose a reason for hiding this comment

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

looks goods, just want to continue the conversation about BackendEnvironment before approving.


@ViewBuilder private var submitButton: some View {
Button(action: {
viewModel.submitPassword(password)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would expect that we submit not only the account password but also the proxy credentials it there are any.

@datadog-wireapp
Copy link

Datadog Report

Branch report: feat/authentication-on-premise-WPB-15225
Commit report: 4bb6d82
Test service: wire-ios-mono

✅ 0 Failed, 27 Passed, 0 Skipped, 9.57s Total Time

Base automatically changed from feat/authentication-ui-cloud-password-unclaimed-v2 to develop February 14, 2025 13:25
Copy link
Contributor

Choose a reason for hiding this comment

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

The heights of the text fields doesn't look right.
Probably @ScaledMetric shouldn't be used.

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.

3 participants