-
Notifications
You must be signed in to change notification settings - Fork 121
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
Conversation
Generated by 🚫 Danger Swift against 3c8654e |
Codecov ReportPatch coverage:
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
Flags with carried forward coverage won't be shown. Click here to find out 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. |
There was a problem hiding this 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?
ElementX/Sources/Screens/InviteUsersInRoom/InviteUsersInRoomCoordinator.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsersInRoom/InviteUsersInRoomModels.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsersInRoom/InviteUsersInRoomModels.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsersInRoom/InviteUsersInRoomModels.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsersInRoom/View/InviteUsersInRoomScreen.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsersInRoom/View/InviteUsersInRoomScreen.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsersInRoom/View/InviteUsersInRoomScreen.swift
Outdated
Show resolved
Hide resolved
What's the purpose of showing the avatars on horizontal scrollable pane? Isn't it just duplication of information? |
There was a problem hiding this 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 :)
ElementX/Sources/Screens/InviteUsers/InviteUsersViewModel.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/InviteUsersViewModel.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/View/InviteUsersScreen.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/View/InviteUsersScreen.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/View/SelectableUserCell.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/View/SelectedInvitedUserItem.swift
Outdated
Show resolved
Hide resolved
UITests/Sources/__Snapshots__/Application/en-GB-iPad-9th-generation.inviteUsers-1.png
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/View/SelectedInvitedUserItem.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/InviteUsersCoordinator.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/InviteUsersViewModel.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👏
ElementX/Sources/Screens/InviteUsers/View/InviteUsersScreen.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
ElementX/Sources/Screens/InviteUsers/InviteUsersCoordinator.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/View/InviteUsersScreen.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/InviteUsers/View/InviteUsersSelectedItem.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/SearchUsersSection/SearchUsersCell.swift
Outdated
Show resolved
Hide resolved
UITests/Sources/__Snapshots__/Application/pseudo-iPhone-14.inviteUsers-1.png
Outdated
Show resolved
Hide resolved
4fc6fd4
to
a67b4aa
Compare
Kudos, SonarCloud Quality Gate passed!
|
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