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

Show just-posted replies above OP replies #4901

Merged
merged 3 commits into from
Aug 8, 2024

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Aug 8, 2024

Follow-up to #4882.

Normally we show OP replies above all so that threads work. However, if you have just posted a reply, it is confusing to have it appear offscreen. Twitter handles this by prioritizing just-posted own replies above the OP's (but showing them below the OP's if you navigate away and back to the post). This accomplishes the same thing.

We can do this now because #4898 consolidated the logic for "after post" callback in the post thread component.

Test Plan

Normal mode:

1111.mov

Thread mode:

222.mov

Note that we only do this when we're sure we won't completely displace the OP post out of view:

333.mov

Concretely, this means that we only do this in threaded more or directly under the highlighted post. Otherwise, it feels too confusing to have the OP post disappear.

Copy link

render bot commented Aug 8, 2024

Copy link
Member

@mozzius mozzius left a comment

Choose a reason for hiding this comment

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

Seems solid in theory

Copy link

github-actions bot commented Aug 8, 2024

Old size New size Diff
7.09 MB 7.09 MB 412 B (0.01%)

@gaearon gaearon merged commit e782db3 into main Aug 8, 2024
6 checks passed
@gaearon gaearon deleted the postthread-prioritize-just-posted branch August 8, 2024 18:20
estrattonbailey added a commit that referenced this pull request Aug 8, 2024
* 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)
  ...
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.

2 participants