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

Select members before creating a room (UI for selection) #766

Merged
merged 22 commits into from
Apr 17, 2023

Conversation

flescio
Copy link
Contributor

@flescio flescio commented Apr 5, 2023

Add screen for inviting users in room, inside the creation room flow, it will also be the same for inviting users in already created room.
The focus is on the selection UI style cell and an horizontal carousel with selected users.

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-04-05.at.14.35.32.mp4

@github-actions
Copy link

github-actions bot commented Apr 5, 2023

Warnings
⚠️ This pull request seems relatively large. Please consider splitting it into multiple smaller ones.
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.

Generated by 🚫 Danger Swift against 3c8654e

@codecov
Copy link

codecov bot commented Apr 5, 2023

Codecov Report

Patch coverage: 14.15% and project coverage change: -0.56 ⚠️

Comparison is base (3d0d883) 52.07% compared to head (b7c4836) 51.52%.

❗ Current head b7c4836 differs from pull request most recent head 3c8654e. Consider uploading reports for the commit 3c8654e to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #766      +/-   ##
===========================================
- Coverage    52.07%   51.52%   -0.56%     
===========================================
  Files          297      295       -2     
  Lines        16544    16224     -320     
  Branches      9923     9829      -94     
===========================================
- Hits          8616     8360     -256     
+ Misses        7678     7622      -56     
+ Partials       250      242       -8     
Flag Coverage Δ
unittests 25.47% <14.15%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
ElementX/Sources/Other/AvatarSize.swift 87.23% <0.00%> (-6.10%) ⬇️
...tX/Sources/Other/SwiftUI/Views/LoadableImage.swift 83.01% <0.00%> (ø)
...s/Screens/InviteUsers/InviteUsersCoordinator.swift 0.00% <0.00%> (ø)
...s/Screens/InviteUsers/View/InviteUsersScreen.swift 0.00% <0.00%> (ø)
.../Screens/InviteUsers/View/SelectableUserCell.swift 0.00% <0.00%> (ø)
...ens/InviteUsers/View/SelectedInvitedUserItem.swift 0.00% <0.00%> (ø)
...creens/SearchUsersSection/SearchUsersSection.swift 0.00% <0.00%> (ø)
...urces/Screens/StartChat/StartChatCoordinator.swift 34.37% <0.00%> (-18.01%) ⬇️
...urces/Screens/StartChat/View/StartChatScreen.swift 76.76% <0.00%> (-0.79%) ⬇️
...vices/UserSession/UserSessionFlowCoordinator.swift 41.26% <0.00%> (+1.41%) ⬆️
... and 3 more

... and 32 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@flescio flescio marked this pull request as ready for review April 5, 2023 09:20
@flescio flescio requested a review from a team as a code owner April 5, 2023 09:20
@flescio flescio requested review from stefanceriu and removed request for a team April 5, 2023 09:20
@flescio flescio enabled auto-merge (squash) April 5, 2023 09:30
@pixlwave pixlwave requested review from pixlwave and removed request for stefanceriu April 5, 2023 10:32
@flescio flescio disabled auto-merge April 5, 2023 13:48
Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

Nice, its great to start seeing this flow 😎

I've added some initial comments, I think it will probably create enough changes to make it a first round for review if that's ok?

@flescio flescio requested review from alfogrillo and pixlwave April 7, 2023 10:24
@ifeeltiredboss
Copy link

What's the purpose of showing the avatars on horizontal scrollable pane? Isn't it just duplication of information?

Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

Thanks, the refactor to InviteUsersXYZ makes this much cleaner 🙏

Nothing major now, just general comments :)

@flescio flescio linked an issue Apr 12, 2023 that may be closed by this pull request
@flescio flescio requested review from pixlwave and alfogrillo April 13, 2023 15:37
Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

LGTM 👏

Copy link
Contributor

@alfogrillo alfogrillo left a comment

Choose a reason for hiding this comment

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

LGTM, just left minor comments

@flescio flescio enabled auto-merge (squash) April 17, 2023 07:18
@flescio flescio disabled auto-merge April 17, 2023 07:34
@flescio flescio force-pushed the flescio/select-invite-users branch from 4fc6fd4 to a67b4aa Compare April 17, 2023 07:38
@flescio flescio enabled auto-merge (squash) April 17, 2023 07:49
@flescio flescio disabled auto-merge April 17, 2023 07:50
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@flescio flescio enabled auto-merge (squash) April 17, 2023 08:25
@flescio flescio merged commit 2b6392e into develop Apr 17, 2023
@flescio flescio deleted the flescio/select-invite-users branch April 17, 2023 08:29
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.

Select members before creating a room (UI for selection)
4 participants