-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[MBL-1386][MBL-1388] Show survey in activity #2058
Conversation
Generated by 🚫 Danger |
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 a question about whether we can remove FindFrendsFacebookConnectCell
and FindFriendsHeaderCell
completely. I don't think they're used anywhere else. Messing with the Activity Storyboard might be out of scope for this PR.
internal func facebookConnect(source: FriendsSource, visible: Bool) { | ||
self.set( | ||
values: visible ? [source] : [], | ||
cellClass: FindFriendsFacebookConnectCell.self, |
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.
Can we remove the FindFrendsFacebookConnectCell altogether? Or is it being used somewhere else?
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.
Removing the cell might mean that the Activity storyboard needs to be updated, which could get wonky....cause storyboards
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.
No, this one is being used elsewhere. It's likely code that we're not actually using, but I'd need to dig through discover and settings to confirm, so that seems out of scope for this pr.
internal func findFriends(source: FriendsSource, visible: Bool) { | ||
self.set( | ||
values: visible ? [source] : [], | ||
cellClass: FindFriendsHeaderCell.self, |
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.
Same question here. Can FindFriendsHeaderCell be removed?
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.
Good call. Deleted!
self.unansweredSurveyResponse.assertValues( | ||
[[surveyResponse], [surveyResponse]], | ||
"Survey emits whenever view appears" | ||
) |
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 assert!
a3efb55
to
8123182
Compare
📲 What
Show unanswered survey cards in activity and delete related code that would show facebook cards.
👀 See
Jira ticket for showing survey
Jira ticket for hiding facebook
Note: Neither of these facebook cards showed before and they're still not going to show. The only change in this pr is that they're actually deleted.
✅ Acceptance criteria