-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Use the suggested actors in the follow suggestion interstitial after a follow #4889
Conversation
Your Render PR Server URL is https://social-app-pr-4889.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cqq5gatds78s73d14ar0. |
|
.map(a => a.did) | ||
.slice(0, 8) | ||
userActionHistory.followSuggestion(dids) | ||
}) |
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.
Should we add a catch here and ignore the error?
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.
I think it'll be fine either way since it's async — we'll see an unhandled rejection at most (which should be fine on both web and RN). Is that correct?
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.
I'd prefer not to ignore in case we introduce a coding mistake somewhere. E.g. if there's a crash, I'd rather it get surfaced. And if we add a catch() that just calls the logger, that seems equivalent to not adding it at all. Lmk if this is wrong 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.
Note that since it's not awaited, a network error wouldn't propagate to the calling code
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.
Love this!
* origin/main: (30 commits) Show just-posted replies above OP replies (#4901) Remove client filtering of starter packs (#4753) Remove show_avi_follow_button (#4900) Remove native_pwi_disabled (#4896) Fix overflow on posts (#4899) Move onPressReply into child component (#4898) Remove new_user_progress_guide (#4895) Remove explore_page_profile_card_social_proof (#4894) Remove ungroup_follow_backs gate (#4893) Remove unnecessary state update for reply gate (#4897) Include follow-based suggestions in interstitial (#4889) Cleanup flags (#4891) ALF suggested follows in profile header (#4828) Added trans (#4890) Keep interstitial fresh on refresh (#4888) Include popcluster in suggestion ranking (#4887) Add logging of selected feed preference when displaying the following feed (#4789) [Video] Visibility detection view (#4741) [Videos] Video player - PR #2 - better web support (#4732) [Video] Authed video upload (#4885) ...
(cherry picked from commit 2174fee)
This incorporates a signal: if you follow someone, we'll show their "suggested by actor" results next time you see the interstitial during this session.
Test Plan
Verify people from the endpoint show up in the suggestions after next Discover refresh.
Note the result is better under the new gate, but the logic should work either way.
m.mov