-
Notifications
You must be signed in to change notification settings - Fork 532
Poll editing and undisclosed polls #5378
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
Conversation
…displaying them in the timeline.
|
📱 Scan the QR code below to install the build for this PR. If you can't scan the QR code you can install the build via this link: https://i.diawi.com/c3P6b9 |
pixlwave
left a comment
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.
Comments inline. Looks great!
PollTimeline -> TimelinePoll = 😎
RiotSwiftUI/Modules/Room/LocationSharing/LocationSharingScreenState.swift
Show resolved
Hide resolved
RiotSwiftUI/Modules/Room/PollEditForm/PollEditFormScreenState.swift
Outdated
Show resolved
Hide resolved
RiotSwiftUI/Modules/Room/PollEditForm/PollEditFormViewModel.swift
Outdated
Show resolved
Hide resolved
RiotSwiftUI/Modules/Room/PollEditForm/Test/Unit/PollEditFormViewModelTests.swift
Outdated
Show resolved
Hide resolved
RiotSwiftUI/Modules/Room/TimelinePoll/Coordinator/TimelinePollCoordinator.swift
Outdated
Show resolved
Hide resolved
RiotSwiftUI/Modules/Room/TimelinePoll/View/TimelinePollAnswerOptionButton.swift
Outdated
Show resolved
Hide resolved
| internal static let pollTypeCheckboxDefault = ImageAsset(name: "poll_type_checkbox_default") | ||
| internal static let pollTypeCheckboxSelected = ImageAsset(name: "poll_type_checkbox_selected") |
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.
Cheeky suggestion, if we were to use circle and record.circle/smallcircle.fill.circle SF symbols instead of custom assets, then they would scale perfectly with dynamic type 😉
Probably a suggestion for @gaelledel rather than you though.
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.
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.
RiotSwiftUI/Modules/Room/TimelinePoll/Coordinator/TimelinePollCoordinator.swift
Show resolved
Hide resolved
64c91b5 to
469bf6d
Compare
8e2c49b to
7c5f5a1
Compare
pixlwave
left a comment
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 📊



How it looks like: