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

Details screen of a poll (PSG-1039) #7314

Merged
merged 19 commits into from
Feb 1, 2023
Merged

Details screen of a poll (PSG-1039) #7314

merged 19 commits into from
Feb 1, 2023

Conversation

flescio
Copy link
Contributor

@flescio flescio commented Jan 30, 2023 •
edited
Loading

Details of poll starting from poll history list.
The poll it's showed accordingly with it's status.
It's done by embedding the existing TimelinePoll in order to save all the existing logic for voting, and live update.
There is the possibility to see the Poll in timeline, navigating back to Room

ezgif-1-75dec60a5b

Flavio Alescio added 12 commits January 23, 2023 16:38
added poll detail scene with mock data
0437838
embedding swiftUI view
bbd5fb4
some fix for models
026ce4a
added real event
4567e54
added view poll in timeline button
96538f8
Merge branch 'develop' into flescio/1040-poll_detail
304ae70 
# Conflicts:
#	Riot/Assets/en.lproj/Vector.strings
#	Riot/Generated/Strings.swift
fix strings after merge
f4c9e66
added view in timeline action, added tests
762af03
Merge branch 'develop' into flescio/1040-poll_detail
5184153 
# Conflicts:
#	Riot/Generated/Strings.swift
#	RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift
#	RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift
#	RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift
generated String
6330ea4
added alert to show possible error, improved tests
7e9f44a
added changelog
2b08aa6
@flescio flescio requested a review from alfogrillo January 30, 2023 09:31
@flescio flescio changed the title PSG-1039 Details screen of a poll (PSG-1039) Jan 30, 2023
@codecov
Copy link

codecov bot commented Jan 30, 2023 •
edited
Loading

Codecov Report

Base: 12.02% // Head: 12.06% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (99a5fee) compared to base (ff6fab7).
Patch coverage: 36.48% of modified lines in pull request are covered.

❗ Current head 99a5fee differs from pull request most recent head 593dbab. Consider uploading reports for the commit 593dbab to get more accurate results

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #7314      +/-   ##
===========================================
+ Coverage    12.02%   12.06%   +0.04%     
===========================================
  Files         1630     1634       +4     
  Lines       161272   161473     +201     
  Branches     66073    66221     +148     
===========================================
+ Hits         19391    19489      +98     
- Misses      141237   141336      +99     
- Partials       644      648       +4
Flag Coverage Δ
uitests 55.78% <77.08%> (+0.14%) ⬆️

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

Impacted Files Coverage Δ
Config/BuildSettings.swift 43.07% <ø> (ø)
Riot/Categories/MXRoom+Riot.m 0.00% <0.00%> (ø)
...iot/Modules/Common/Recents/RecentsViewController.m 8.70% <0.00%> (ø)
...Modules/Common/Recents/Views/RecentTableViewCell.m 0.00% <0.00%> (ø)
...ntextMenu/ActionProviders/RoomActionProvider.swift 0.00% <0.00%> (ø)
...ontextMenu/Services/RoomContextActionService.swift 0.00% <0.00%> (ø)
...ontrollers/MXKNotificationSettingsViewController.m 0.00% <0.00%> (ø)
...ules/MatrixKit/Models/RoomList/MXKRecentCellData.m 0.00% <0.00%> (ø)
...atrixKit/Views/PushRule/MXKPushRuleTableViewCell.m 0.00% <0.00%> (ø)
Riot/Modules/Room/MXKRoomViewController.m 0.00% <0.00%> (ø)
... and 42 more

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

alfogrillo
alfogrillo reviewed Jan 30, 2023
View reviewed changes
Riot/Modules/Room/RoomInfo/RoomInfoCoordinator.swift Outdated Show resolved Hide resolved
Riot/Modules/Room/RoomViewController.m Outdated Show resolved Hide resolved
Riot/Modules/Spaces/SpaceRoomList/ExploreRoomCoordinator.swift Outdated Show resolved Hide resolved
RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift Outdated Show resolved Hide resolved
RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift Outdated Show resolved Hide resolved
RiotSwiftUI/Modules/Room/PollHistory/PollHistoryDetail/View/PollHistoryDetail.swift Outdated Show resolved Hide resolved
...I/Modules/Room/PollHistory/PollHistoryDetail/Test/Unit/PollHistoryDetailViewModelTests.swift Show resolved Hide resolved
RiotSwiftUI/Modules/Room/PollHistory/PollHistoryDetail/View/PollHistoryDetail.swift Outdated Show resolved Hide resolved
RiotSwiftUI/Modules/Room/PollHistory/PollHistoryDetail/View/PollHistoryDetail.swift Outdated Show resolved Hide resolved
RiotSwiftUI/Modules/Room/PollHistory/PollHistoryDetail/View/PollHistoryDetail.swift Outdated Show resolved Hide resolved
alfogrillo
alfogrillo approved these changes Jan 31, 2023
View reviewed changes
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! 🎉

Riot/Modules/Room/RoomViewController.m Outdated Show resolved Hide resolved
stefanceriu
stefanceriu requested changes Jan 31, 2023
View reviewed changes
...UI/Modules/Room/PollHistory/PollHistoryDetail/Coordinator/PollHistoryDetailCoordinator.swift Outdated
self.parameters = parameters
let timelinePollCoordinator = try TimelinePollCoordinator(parameters: .init(session: parameters.room.mxSession, room: parameters.room, pollEvent: parameters.event))

let viewModel = PollHistoryDetailViewModel(timelineViewModel: timelinePollCoordinator.viewModel)
Copy link
Member

Choose a reason for hiding this comment

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

What's the idea behind constructing a viewModel with another viewModel? There should be a 1to1 relationship between a coordinator a view model and the view it controls

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 1, 2023

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
No Duplication information No Duplication information

@stefanceriu stefanceriu self-requested a review February 1, 2023 14:34
@flescio flescio merged commit 84ecbf6 into develop Feb 1, 2023
@flescio flescio deleted the flescio/1040-poll_detail branch February 1, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alfogrillo alfogrillo alfogrillo approved these changes

@stefanceriu stefanceriu stefanceriu approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@flescio @stefanceriu @alfogrillo