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.

Sign up for GitHub

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

Jump to bottom

feat: create PasswordField component in WireUI/WireReusableUIComponent - WPB-15537 #2406

Merged
merged 11 commits into from
Jan 24, 2025
Merged

feat: create PasswordField component in WireUI/WireReusableUIComponent - WPB-15537 #2406

merged 11 commits into from
Jan 24, 2025

Conversation

El-Fitz
Copy link
Contributor

@El-Fitz El-Fitz commented Jan 21, 2025 •
edited
Loading

Issue

Creates a new PasswordField reusable component, based on the PasswordFieldView component from #2339.

Testing

See SwiftUI previews.

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.

@El-Fitz El-Fitz changed the title feat: create PasswordField component in WireUI/WireReusableUIComponent feat: create PasswordField component in WireUI/WireReusableUIComponent - WPB-15537 Jan 21, 2025
@El-Fitz El-Fitz marked this pull request as draft January 21, 2025 12:06
@El-Fitz El-Fitz requested a review from caldrian January 21, 2025 12:08
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 21, 2025 •
edited
Loading

Test Results

  1 files    9 suites   34s ⏱️
105 tests 104 ✅ 1 💤 0 ❌
106 runs  105 ✅ 1 💤 0 ❌

Results for commit 4fd73d0.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jan 21, 2025 •
edited
Loading

Datadog Report

Branch report: feat/extract-passwordfieldview-to-reusable-components
Commit report: 249f6e0
Test service: wire-ios-mono

❌ 6 Failed (0 Known Flaky), 76 Passed, 1 Skipped, 10.7s Total Time

❌ Failed Tests (6)

This report shows up to 5 failed tests.

  • testColorSchemeVariants() - WireReusableUIComponentsTests

  • testDynamicTypeVariants() - WireReusableUIComponentsTests - Details

    Expand for error 
     XCTAssertNil failed: "ERR260.DFV" (/Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireReusableUIComponentsTests/PasswordField/PasswordFieldSnapshotTests.swift#EndingLineNumber=118&StartingLineNumber=118)

  • testInvalidHidden() - WireReusableUIComponentsTests - Details

    Expand for error 
     XCTAssertNil failed: "ERR260.DFV" (/Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireReusableUIComponentsTests/PasswordField/PasswordFieldSnapshotTests.swift#EndingLineNumber=54&StartingLineNumber=54)

  • testInvalidVisible() - WireReusableUIComponentsTests - Details

    Expand for error 
     XCTAssertNil failed: "ERR260.DFV" (/Users/admin/actions-runner/_work/wire-ios/wire-ios/WireUI/Tests/WireReusableUIComponentsTests/PasswordField/PasswordFieldSnapshotTests.swift#EndingLineNumber=65&StartingLineNumber=65)

  • testValidHidden() - WireReusableUIComponentsTests

caldrian
caldrian reviewed Jan 21, 2025
View reviewed changes
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.

Not sure if this is ready to be reviewed. The PR is still a draft.

WireUI/Sources/WireReusableUIComponents/Resources/en.lproj/Localizable.strings Outdated Show resolved Hide resolved
caldrian
caldrian approved these changes Jan 21, 2025
View reviewed changes
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.

👍

@El-Fitz El-Fitz force-pushed the feat/extract-passwordfieldview-to-reusable-components branch from 3d3e2af to 813b138 Compare January 22, 2025 13:49
@El-Fitz El-Fitz marked this pull request as ready for review January 22, 2025 13:52
@El-Fitz El-Fitz force-pushed the feat/extract-passwordfieldview-to-reusable-components branch from bd41302 to 94a019a Compare January 22, 2025 13:52
@El-Fitz
Copy link
Contributor Author

El-Fitz commented Jan 22, 2025

A separate ticket, WPB-15571, has been created for accessibility strings.

@KaterinaWire
Copy link
Contributor

@El-Fitz There is an issue that the "eye" icon overlaps the password for large fonts. This can be seen in the snapshots

@El-Fitz El-Fitz requested a review from KaterinaWire January 22, 2025 16:05
netbe
netbe reviewed Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@netbe netbe left a comment

Choose a reason for hiding this comment

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

I left a couple of comments for now, but it looks good

WireUI/Sources/WireReusableUIComponents/PasswordTextField/PasswordField.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireReusableUIComponents/PasswordTextField/PasswordField.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireReusableUIComponents/PasswordTextField/PasswordField.swift Outdated Show resolved Hide resolved
...ources/ReferenceImages/PasswordFieldSnapshotTests/testDynamicTypeVariants.accessibility4.png Outdated Show resolved Hide resolved
@KaterinaWire
Copy link
Contributor

@El-Fitz Snapshot tests are missing

@samwyndham
Copy link
Contributor

Un-assigning myself as this PR already has multiple reviewers. Feel free to add me back @El-Fitz if you would like

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jan 24, 2025 •
edited
Loading

Datadog Report

Branch report: feat/extract-passwordfieldview-to-reusable-components
Commit report: 64b4f9a
Test service: wire-ios-mono

✅ 0 Failed, 82 Passed, 1 Skipped, 19.17s Total Time

@samwyndham samwyndham removed their request for review January 24, 2025 10:05
@KaterinaWire KaterinaWire requested a review from netbe January 24, 2025 10:34
@El-Fitz El-Fitz force-pushed the feat/extract-passwordfieldview-to-reusable-components branch from 35466c0 to bafdd72 Compare January 24, 2025 14:18
@El-Fitz El-Fitz force-pushed the feat/extract-passwordfieldview-to-reusable-components branch from bafdd72 to 7cce354 Compare January 24, 2025 14:19
@El-Fitz El-Fitz added this pull request to the merge queue Jan 24, 2025
Merged via the queue into develop with commit 6c08e8d Jan 24, 2025
10 of 11 checks passed
@El-Fitz El-Fitz deleted the feat/extract-passwordfieldview-to-reusable-components branch January 24, 2025 15:51
@echoes-hq echoes-hq bot added the echoes: product-roadmap/feature Work contributing to adding a new feature as part of the product roadmap. label Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KaterinaWire KaterinaWire KaterinaWire approved these changes

@caldrian caldrian caldrian approved these changes

@jullianm jullianm Awaiting requested review from jullianm

@netbe netbe Awaiting requested review from netbe

Assignees

@El-Fitz El-Fitz

Labels
echoes: product-roadmap/feature Work contributing to adding a new feature as part of the product roadmap.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@El-Fitz @KaterinaWire @samwyndham @netbe @caldrian