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: individual to team migration ui - WPB-10347 #2135

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

feat: individual to team migration ui - WPB-10347 #2135

wants to merge 7 commits into from
+1,579 −280

Conversation

El-Fitz
Copy link
Contributor

@El-Fitz El-Fitz commented Nov 7, 2024 •
edited
Loading

Issue

Implement the UI for individual to team migration.

  • Some colors need to be changed to use either WireDesign.ColorTheme or be constructed using WireDesign.BaseColorPalette.
  • Accessibility strings need to be added.

Testing

The SwiftUI previews can be found in WireUI/Sources/WireIndividualToTeamMigrationUI/Views/Previews.
Use WireUI/Sources/WireIndividualToTeamMigrationUI/Views/Previews/_FeaturePreview to test the whole flow.

Screenshot 2024-11-12 at 18 47 45
Screenshot 2024-11-12 at 18 47 49
Screenshot 2024-11-12 at 18 47 54
Screenshot 2024-11-12 at 18 48 01
Screenshot 2024-11-12 at 18 48 08
Screenshot 2024-11-12 at 18 48 12
Screenshot 2024-11-12 at 18 48 16

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 self-assigned this Nov 7, 2024
netbe
netbe reviewed Nov 8, 2024
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.

Looks good, adding a few comments as a first feedback before approval

WireUI/Sources/WireIndividualToTeamMigrationUI/Components/BackButton.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/CallToActionButton.swift Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/FeatureDescriptionComponent.swift Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/Checkbox.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/PageContainer.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Strings.swift

import Foundation

extension AttributedString {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we would like to reuse this on other modules too, would WireFoundation be appropriate ? cc @caldrian wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should we do, @netbe ?

WireUI/Sources/WireIndividualToTeamMigrationUI/Views/CompletionView.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Views/CompletionView.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Views/ConfirmationView.swift

var body: some View {
VStack(alignment: .leading, spacing: 24) {
Text(String.localized(key: "individualToTeam.confirmation.body", bundle: .module))
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: do we have to specify .module here or is the default looking at the localizable table of the module?

Copy link
Contributor Author

@El-Fitz El-Fitz Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Because the documentation for String.init(localized:table:bundle:locale:comment:) states that if the bundle parameter is nil (and it defaults to nil), "an app searches its main bundle", which obviously doesn't contain our module's strings.

But our method could default to .module. It really depends on how you intend to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you want to do @netbe ?

WireUI/Tests/WireIndividualToTeamMigrationUITests/WireIndividualToTeamMigrationTests.swift Show resolved Hide resolved
@netbe
Copy link
Collaborator

netbe commented Nov 8, 2024

@El-Fitz be careful I think the design have been slightly updated in Figma: i.e replaced the backbutton with navigation back button

@El-Fitz
Copy link
Contributor Author

El-Fitz commented Nov 12, 2024

@El-Fitz be careful I think the design have been slightly updated in Figma: i.e replaced the backbutton with navigation back button

Yes, I updated the UI. I'll push those changes and update the screenshots.

@El-Fitz
Copy link
Contributor Author

El-Fitz commented Nov 12, 2024

The screenshots have been updated, and you can use the preview in WireUI/Sources/WireIndividualToTeamMigrationUI/Views/Previews/_FeaturePreview to test the whole flow.

@wireapp wireapp deleted a comment from ThomasLeger-x-Luni Nov 12, 2024
agisilaos
agisilaos reviewed Nov 13, 2024
View reviewed changes
Copy link
Contributor

@agisilaos agisilaos left a comment

Choose a reason for hiding this comment

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

Left some questions regarding the colors we use and a couple of suggestions! Thanks @El-Fitz!

WireUI/Sources/WireIndividualToTeamMigrationUI/Components/BackButton.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/BackButton.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/CallToActionButton.swift Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/CallToActionButton.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/Checkbox.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/Components/PageContainer.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/IndividualToTeamMigrationViewController.swift Outdated Show resolved Hide resolved
WireUI/Sources/WireIndividualToTeamMigrationUI/IndividualToTeamMigrationViewController.swift Outdated Show resolved Hide resolved
@El-Fitz
Copy link
Contributor Author

El-Fitz commented Nov 14, 2024

@agisilaos Should be all done, regarding the colors & accessibility labels.

@El-Fitz El-Fitz force-pushed the feat/individual-to-team-migration-ui branch from 12300f5 to 7ebdca4 Compare November 15, 2024 09:09
@github-actions GitHub Actions
Copy link
Contributor

Test Results

1 tests   1 ✅  0s ⏱️
1 suites  0 💤
2 files    0 ❌

Results for commit 7ebdca4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agisilaos agisilaos agisilaos left review comments

@netbe netbe netbe left review comments

@caldrian caldrian Awaiting requested review from caldrian

At least 2 approving reviews are required to merge this pull request.

Assignees

@El-Fitz El-Fitz

Labels
echoes/initiative: personal-users-to-team-users...
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@El-Fitz @netbe @agisilaos