-
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
Tweak feed card to prevent spinnerz when pushing to screen #4600
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
types lint tweak to prevent spinnerzzz state tweak feed card to not required a did resolve req
Your Render PR Server URL is https://social-app-pr-4600.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cpr31iss1f4s73b9fi70. |
haileyok
changed the title
tweak to prevent spinnerzzz state
Tweak feed card to prevent spinnerz when pushing to screen
Jun 22, 2024
|
gaearon
reviewed
Jun 22, 2024
gaearon
reviewed
Jun 22, 2024
gaearon
reviewed
Jun 22, 2024
gaearon
reviewed
Jun 22, 2024
gaearon
approved these changes
Jun 22, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
There's some spinner states on feed cards - in prod, not related to the
FeedCard
change necessarily - that we'd like to remove. Easy little win.We also didn't actually migrate to using the new
FeedCard
in theProfileFeedgens
list. I had done this work in the starterpacks branch, so I picked it out of there and made a couple tweaks after the recentFeedCard
merge into main.Finally, made a little types tweak inside of
FeedCard
so thatLink
knows whether something is aListView
orGeneratorView
, supporting this new precaching.How
We already have a
precacheFeed
function that takes inFeedSourceInfo
from inside one of our query functions. However, we also want to make it possible to run this with aGeneratorView
.Following a similar pattern that we use for posts and profiles, we're adding a
precacheFeedFromGeneratorView
that can be called with theLink
'sonPress
. This runs just before pushing to the screen - and lets the query already have the info it needs.Test Plan
Visit a profile that has some feeds, and notice the lack of a spinner state (easiest to see with throttling)
Before
Screen.Recording.2024-06-21.at.7.03.42.PM.mov
After
Screen.Recording.2024-06-21.at.7.04.49.PM.mov