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: create authentication identity input view - WPB-15221 #2343

Merged
merged 19 commits into from
Jan 27, 2025

Conversation

El-Fitz
Copy link
Contributor

@El-Fitz El-Fitz commented Jan 3, 2025

StoryWPB-15221 [iOS] Implement new authentication flow views - Identification View

Issue

Create the email input view

Testing

SwiftUI previews have been provided.


Checklist

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

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.

Copy link
Contributor

github-actions bot commented Jan 3, 2025

Test Results

  1 files   10 suites   27s ⏱️
107 tests 106 ✅ 1 💤 0 ❌
108 runs  107 ✅ 1 💤 0 ❌

Results for commit b050b89.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jan 3, 2025

Datadog Report

Branch report: feat/authentication-ui-identity-input
Commit report: 372180a
Test service: wire-ios-mono

❌ 2 Failed (0 Known Flaky), 76 Passed, 1 Skipped, 1m 26.06s Total Time

❌ Failed Tests (2)

  • testColorSchemeVariants() - WireAuthenticationUITests - Details

    Expand for error
     XCTAssertNil failed: "Snapshot "light" does not match reference.
     @−
     "file:///Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireAuthenticationUITests/Resources/ReferenceImages/AuthenticationIdentityInputViewTests/testColorSchemeVariants.light.png"
     @+
     "file:///Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireAuthenticationUITests/Resources/SnapshotResults/AuthenticationIdentityInputViewTests/testColorSchemeVariants.light.png"
     To configure output for a custom diff tool, use 'withSnapshotTesting'. For example:
         withSnapshotTesting(diffTool: .ksdiff) {
           // ...
         }
     Newly-taken snapshot does not match reference." (/Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireAuthenticationUITests/AuthenticationIdentityInputViewTests.swift#EndingLineNumber=45&StartingLineNumber=45)
    
  • testDynamicTypeVariants() - WireAuthenticationUITests - Details

    Expand for error
     XCTAssertNil failed: "Snapshot "xSmall" does not match reference.
     @−
     "file:///Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireAuthenticationUITests/Resources/ReferenceImages/AuthenticationIdentityInputViewTests/testDynamicTypeVariants.xSmall.png"
     @+
     "file:///Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireAuthenticationUITests/Resources/SnapshotResults/AuthenticationIdentityInputViewTests/testDynamicTypeVariants.xSmall.png"
     To configure output for a custom diff tool, use 'withSnapshotTesting'. For example:
         withSnapshotTesting(diffTool: .ksdiff) {
           // ...
         }
     Newly-taken snapshot does not match reference." (/Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireAuthenticationUITests/AuthenticationIdentityInputViewTests.swift#EndingLineNumber=60&StartingLineNumber=60)
    

@caldrian caldrian changed the title feat: create authentication identity input view - WPB15221 feat: create authentication identity input view - WPB-15221 Jan 3, 2025
Copy link
Contributor

@caldrian caldrian left a comment

Choose a reason for hiding this comment

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

Please create a snapshot test like SidebarMenuItemSnapshotTests.

Copy link
Contributor

@samwyndham samwyndham left a comment

Choose a reason for hiding this comment

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

Personally I would have this PR targeting develop and additionally ensure that all strings are localized. That way it doesn't need to be reviewed again and we have a smaller ongoing feature branch. What do you think?

Sorry I misunderstood. Please ignore this comment

Copy link
Contributor

@samwyndham samwyndham left a comment

Choose a reason for hiding this comment

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

Please add localized strings, and run swiftlint/ swiftformat.

@El-Fitz El-Fitz force-pushed the feat/authentication-ui-background branch from 9713e5b to 37db2c1 Compare January 7, 2025 17:53
@El-Fitz El-Fitz force-pushed the feat/authentication-ui-identity-input branch from 6449d7e to 6732375 Compare January 7, 2025 18:00
@El-Fitz El-Fitz force-pushed the feat/authentication-ui-background branch from a76922a to 7564aee Compare January 8, 2025 05:39
@El-Fitz El-Fitz force-pushed the feat/authentication-ui-identity-input branch from d8a8e75 to 0509462 Compare January 8, 2025 05:46
Copy link
Contributor

@samwyndham samwyndham left a comment

Choose a reason for hiding this comment

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

Nice work. I've commented about some differences in appearance compared to the designs I see in Figma

@netbe netbe self-requested a review January 17, 2025 11:31
@El-Fitz El-Fitz force-pushed the feat/authentication-ui-background branch from 7564aee to f8bbb94 Compare January 17, 2025 16:02
@El-Fitz El-Fitz force-pushed the feat/authentication-ui-identity-input branch from 3738e21 to f2dba29 Compare January 17, 2025 16:30
Base automatically changed from feat/authentication-ui-background to develop January 17, 2025 18:25
@echoes-hq echoes-hq bot added the echoes/initiative: scale Enterprise Readiness Initiatives label Jan 17, 2025
Copy link
Contributor

@samwyndham samwyndham left a comment

Choose a reason for hiding this comment

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

Nice work!

@El-Fitz El-Fitz requested a review from caldrian January 20, 2025 13:21
@El-Fitz El-Fitz force-pushed the feat/authentication-ui-identity-input branch from f2dba29 to 78c9355 Compare January 20, 2025 13:23
@El-Fitz El-Fitz force-pushed the feat/authentication-ui-identity-input branch from 1375a23 to 98f111a Compare January 24, 2025 15:46
@El-Fitz El-Fitz enabled auto-merge January 27, 2025 15:17
@El-Fitz El-Fitz added this pull request to the merge queue Jan 27, 2025
Merged via the queue into develop with commit 0b9b184 Jan 27, 2025
10 of 11 checks passed
@El-Fitz El-Fitz deleted the feat/authentication-ui-identity-input branch January 27, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes/initiative: scale Enterprise Readiness Initiatives
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants