Skip to content
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

Remove getProfile calls when loading feed #3881

Merged
merged 6 commits into from
May 23, 2024

Conversation

haileyok
Copy link
Member

@haileyok haileyok commented May 6, 2024

Why

Currently, we make a ton of calls to getProfile() to display the "Reply to" label. However, since bluesky-social/atproto#2461, we can now get this info from the grandparentAuthor field in the API response.

Test Plan

Before

Screenshot 2024-05-06 at 4 20 52 PM

After

Screenshot 2024-05-06 at 4 21 12 PM

Verify that the requests are no longer present when loading your timeline. Also verify that posts properly display the Reply to label when they should.

Check situations where:

  • It appears normally: i.e. a post with one parent above. It should show the parent's parent's author.
  • We remove the parent from the timeline. It should show the parent's author.
  • Where we put a thread together, i.e. below

A normal situation

Screenshot 2024-05-06 at 5 19 31 PM

Removing the parent

Screenshot 2024-05-06 at 5 18 51 PM

Us making a thread

Screenshot 2024-05-06 at 5 16 31 PM

Copy link

render bot commented May 6, 2024

@haileyok haileyok force-pushed the hailey/remove-getprofile-userinfotext branch from f610de3 to fe01005 Compare May 6, 2024 23:31
Copy link

github-actions bot commented May 6, 2024

Old size New size Diff
7.29 MB 7.3 MB 3.77 KB (0.05%)

@haileyok haileyok force-pushed the hailey/remove-getprofile-userinfotext branch from 825f3e6 to 0c16dba Compare May 7, 2024 00:39
@@ -34,6 +34,7 @@ export const DEFAULT_LOGGED_OUT_PREFERENCES: UsePreferencesQueryResponse = {
pinned: [],
unpinned: [],
},
savedFeeds: [],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the correct move here

Copy link
Member

@estrattonbailey estrattonbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just blocking for now on the SDK update, looks great otherwise

package.json Outdated Show resolved Hide resolved
add comments

ensure only if first

simplify

nit

handle cases where the parent is removed

add a comment

remove unnecessary `getProfile()` calls from feed load

limit only to the first post in the returned items

move the logic out of the render and into the query

add the grandparent properly

update `FeedItem`

bump package

update `FeedItem`

update `post-feed` query

update `FeedSlice`
@gaearon gaearon force-pushed the hailey/remove-getprofile-userinfotext branch from 2d7c37b to 87bc522 Compare May 23, 2024 18:02
@gaearon gaearon merged commit 70f190d into main May 23, 2024
6 checks passed
@gaearon gaearon deleted the hailey/remove-getprofile-userinfotext branch May 23, 2024 18:35
estrattonbailey added a commit that referenced this pull request May 23, 2024
* origin/main:
  Remove `getProfile` calls when loading feed (#3881)
  Log error statuses from failed resume session calls (#4174)
  disable alt text auto focus on Android (#4198)
  [🐴] add link to chat settings from main settings (#4197)
  ✍️ Add OTA Docs (#4187)
  Decrease thickness of border on message input (#4196)
  [🐴] better error message for "Bad token scope" error (#4194)
  Add padding to dialogs when keyboard is open on Android (#4182)
  [🐴] Do not init event bus if no session (#4193)
  stop line breaks for timeelapsed (#4191)
  Reduce polling when app is backgrounded (#4192)
  implement a safari hack for ime (#4186)
  [🐴] Suspend event bus when switching accounts (#4190)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants