Active poll history UI (PSG-906) #7267
Conversation
alfogrillo
force-pushed
the
alfogrillo/active_poll_history
branch
from
21689e4
to
c7c3b85
Compare
alfogrillo
requested review from
a team and
pixlwave
and removed request for
a team
Codecov ReportBase: 11.83% // Head: 11.94% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #7267 +/- ##
===========================================
+ Coverage 11.83% 11.94% +0.10%
===========================================
Files 1630 1637 +7
Lines 160317 160581 +264
Branches 65489 65618 +129
===========================================
+ Hits 18977 19184 +207
- Misses 140701 140758 +57
Partials 639 639
Flags with carried forward coverage won't be shown. Click here to find out 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. |
| let enumeratedPolls = Array(viewModel.viewState.polls.enumerated()) | ||
|
|
||
| ForEach(enumeratedPolls, id: \.offset) { _, pollData in |
There was a problem hiding this comment.
It seems more sensible to make PollListData conform to Identifiable (using the poll start event ID presumably) than enumerating them just for an index which is potentially an unstable ID for the poll if the order changed.
There was a problem hiding this comment.
PollListData is totally a mock for now.
This will change a lot in next PRs I guess...
| struct PollListData { | ||
| let startDate: Date | ||
| let question: String | ||
| } |
There was a problem hiding this comment.
Should probably go in PollHistoryModels fwiw
There was a problem hiding this comment.
I fell like dumb data model can stay in the same file of the view that consumes them.
Doing so you have everything you need to work on the component in a single file.
I'm not against moving it btw.
|
|
||
| import SwiftUI | ||
|
|
||
| struct SegmentedPicker<Tag: Hashable>: View { |
There was a problem hiding this comment.
Ah its a shame we can't use a custom PickerStyle here to get voiceover support for free. Looks to be a private API for now 😕
Should probably add the necessary accessibility modifiers to the segments to communicate what is/isn't selected for now I guess.
There was a problem hiding this comment.
Ah its a shame we can't use a custom PickerStyle here to get voiceover support for free. Looks to be a private API for now
Yeah, I tried that before I invented a new component.
Apple you need to do the right thing here! 😄
Added the accessibility labels btw: 5735b5d
RiotSwiftUI/Modules/Room/PollHistory/View/SegmentedPicker.swift
Outdated
Show resolved
Hide resolved
| .font(isSelectedSegment ? theme.fonts.headline : theme.fonts.body) | ||
| .underline(isSelectedSegment) | ||
| } | ||
| .accentColor(isSelectedSegment ? theme.colors.accent : theme.colors.primaryContent) | ||
| .accessibilityLabel(segment.description + (isSelectedSegment ? "-selected" : "")) |
There was a problem hiding this comment.
I might be wrong, but maybe you can use .accessibilityValue here for selected?
There was a problem hiding this comment.
UI tests (like they are now) fail switching to .accessibilityValue.
Why do you think it would be a better option?
There was a problem hiding this comment.
Addressed here -> 25a52a2
|
Kudos, SonarCloud Quality Gate passed!
|
This PR:
Result