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

Create a room screen (UI only) #877

Merged
merged 20 commits into from
May 16, 2023
Merged

Create a room screen (UI only) #877

merged 20 commits into from
May 16, 2023

Conversation

flescio
Copy link
Contributor

@flescio flescio commented May 11, 2023

Add the create room screen, UI only, picking the room image or actually creating a room are out of scope.

Simulator Screenshot - iPhone 14 Pro - 2023-05-11 at 12 29 19

@flescio flescio requested a review from alfogrillo May 11, 2023 10:37
@flescio flescio linked an issue May 11, 2023 that may be closed by this pull request
Create a room screen (UI) element-hq/element-meta#1059
Closed
@github-actions
Copy link

github-actions bot commented May 11, 2023 •
edited
Loading

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.
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 Danger Swift against 7b7b51d

@flescio flescio force-pushed the flescio/create_room_screen branch 2 times, most recently from 999bad1 to b569773 Compare May 11, 2023 12:05
@codecov
Copy link

codecov bot commented May 11, 2023 •
edited
Loading

Codecov Report

Patch coverage: 18.60% and project coverage change: -1.60 ⚠️

Comparison is base (f4bbe33) 50.01% compared to head (085f079) 48.41%.

❗ Current head 085f079 differs from pull request most recent head 7b7b51d. Consider uploading reports for the commit 7b7b51d to get more accurate results

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #877      +/-   ##
===========================================
- Coverage    50.01%   48.41%   -1.60%     
===========================================
  Files          324      329       +5     
  Lines        19603    19872     +269     
  Branches     10882    11004     +122     
===========================================
- Hits          9804     9622     -182     
- Misses        9521     9974     +453     
+ Partials       278      276       -2
Flag Coverage Δ
unittests 22.95% <18.60%> (-0.13%) ⬇️

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

Impacted Files Coverage Δ
ElementX/Sources/Application/AppCoordinator.swift 42.25% <0.00%> (-12.28%) ⬇️
...mentX/Sources/Other/AccessibilityIdentifiers.swift 5.08% <0.00%> (-31.76%) ⬇️
...s/Other/SwiftUI/Form Styles/FormButtonStyles.swift 51.02% <0.00%> (-1.56%) ⬇️
...ces/Screens/CreateRoom/CreateRoomCoordinator.swift 0.00% <0.00%> (ø)
...ces/Screens/CreateRoom/View/CreateRoomScreen.swift 0.00% <0.00%> (ø)
...viteUsersScreen/InviteUsersScreenCoordinator.swift 48.14% <0.00%> (-12.73%) ⬇️
...ns/InviteUsersScreen/InviteUsersScreenModels.swift 92.30% <ø> (ø)
...ens/InviteUsersScreen/View/InviteUsersScreen.swift 60.20% <0.00%> (-1.03%) ⬇️
...s/StartChatScreen/StartChatScreenCoordinator.swift 15.90% <0.00%> (-19.10%) ⬇️
...urces/Screens/CreateRoom/CreateRoomViewModel.swift 95.00% <95.00%> (ø)
... and 4 more

... and 18 files with indirect coverage changes

☔ 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 May 11, 2023 12:57
@flescio flescio requested a review from a team as a code owner May 11, 2023 12:57
@flescio flescio requested review from stefanceriu and removed request for a team May 11, 2023 12:57
@flescio flescio linked an issue May 12, 2023 that may be closed by this pull request
Create a room screen (UI) #629
Closed
@flescio flescio force-pushed the flescio/create_room_screen branch from 832fcdb to d64577b Compare May 12, 2023 08:02
stefanceriu
stefanceriu requested changes May 12, 2023
View reviewed changes
Copy link
Member

@stefanceriu stefanceriu left a comment

Choose a reason for hiding this comment

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

Nice one! I left some comments inline

ElementX/Sources/Other/AccessibilityIdentifiers.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/CreateRoom/CreateRoomViewModel.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/CreateRoom/View/CreateRoomScreen.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/CreateRoom/View/CreateRoomScreen.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/CreateRoom/View/CreateRoomScreen.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/SessionVerificationScreen/SessionVerificationScreenViewModel.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/StartChatScreen/StartChatScreenCoordinator.swift Outdated Show resolved Hide resolved
UITests/Sources/CreateRoomScreenUITests.swift Outdated Show resolved Hide resolved
UnitTests/Sources/CreateRoomViewModelTests.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/CreateRoom/View/CreateRoomScreen.swift Outdated Show resolved Hide resolved
@flescio flescio force-pushed the flescio/create_room_screen branch from d64577b to 2263aae Compare May 12, 2023 12:22
alfogrillo
alfogrillo reviewed May 12, 2023
View reviewed changes
ElementX/Sources/Screens/CreateRoom/CreateRoomViewModel.swift Show resolved Hide resolved
ElementX/Sources/Screens/CreateRoom/CreateRoomViewModel.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/CreateRoom/View/CreateRoomScreen.swift Show resolved Hide resolved
ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenCoordinator.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenCoordinator.swift Outdated Show resolved Hide resolved
ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenCoordinator.swift Outdated Show resolved Hide resolved
ElementX/Sources/Services/CreateRoom/CreateRoomParameters.swift Outdated Show resolved Hide resolved
UITests/Sources/CreateRoomScreenUITests.swift Outdated Show resolved Hide resolved
UITests/Sources/__Snapshots__/Application/en-GB-iPad-9th-generation.createRoom-2.png Outdated Show resolved Hide resolved
@flescio flescio force-pushed the flescio/create_room_screen branch 2 times, most recently from 96f0036 to 30e56b9 Compare May 12, 2023 16:57
pixlwave
pixlwave reviewed May 12, 2023
View reviewed changes
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.

Not a proper review, just looked at the screenshot and noticed the following:

ElementX/Sources/Screens/CreateRoom/View/CreateRoomScreen.swift Show resolved Hide resolved
ElementX/Sources/Screens/CreateRoom/View/CreateRoomScreen.swift Show resolved Hide resolved
@flescio flescio force-pushed the flescio/create_room_screen branch from 30e56b9 to 85273d9 Compare May 15, 2023 08:26
stefanceriu
stefanceriu approved these changes May 15, 2023
View reviewed changes
Copy link
Member

@stefanceriu stefanceriu 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 to me, nicely done 👏

@flescio flescio force-pushed the flescio/create_room_screen branch from 85273d9 to 085f079 Compare May 15, 2023 14:16
@flescio flescio linked an issue May 16, 2023 that may be closed by this pull request
Create a room and show it #630
Closed
@flescio flescio removed a link to an issue May 16, 2023
Create a room and show it #630
Closed
Flavio Alescio and others added 16 commits May 16, 2023 17:49
UI on create room screen
49ba0ff
add test case for room creation
0982901
add UI test, separate model class in Services, UI fix for button with…
a7e779a 
… single selection style
add screenshots for UI test
28c8453
add changelog, fix schreenshots
b6c009f
fix throwing task compile error
066ef1e
change accesibility identifiers, remove single selection button style…
e8fcb36 
…, use Picker instead,
remove create room parameters by reference and use coordinator flow i…
9228e49 
…nstead
fix compile error, Task not throwing
f29bc6f
@alfogrillo
Update ElementX/Sources/Screens/CreateRoom/CreateRoomViewModel.swift
84ceb89 
Co-authored-by: Alfonso Grillo <alfogrillo@gmail.com>
@alfogrillo
Update ElementX/Sources/Screens/CreateRoom/View/CreateRoomScreen.swift
3826ae2 
Co-authored-by: Alfonso Grillo <alfogrillo@gmail.com>
remove keyboard from screenshots
2d3d1bc
UI section title fix
b1bbba6
remove dismiss keyboard from screenshots
ed7317a
fix UI test async
44d2bb3
change on proj
7b7b51d
@flescio flescio force-pushed the flescio/create_room_screen branch from 085f079 to 7b7b51d Compare May 16, 2023 16:09
@flescio flescio enabled auto-merge (rebase) May 16, 2023 16:09
@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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@flescio flescio merged commit 1b4ef0b into develop May 16, 2023
@flescio flescio deleted the flescio/create_room_screen branch May 16, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefanceriu stefanceriu stefanceriu approved these changes

@alfogrillo alfogrillo Awaiting requested review from alfogrillo

@pixlwave pixlwave Awaiting requested review from pixlwave

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create a room screen (UI) Create a room screen (UI)
4 participants
@flescio @stefanceriu @pixlwave @alfogrillo