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

Fix: to post positioning after thread load #1587

Merged
merged 1 commit into from
Oct 4, 2023
Merged

Conversation

pfrazee
Copy link
Collaborator

@pfrazee pfrazee commented Oct 3, 2023

I was noticing some cases where quote posts would position incorrectly after tapping them and loading their thread screen. I believe this is because the index passed to maintainVisibleContentPosition is correct when rendering replies but not when rendering toplevel posts (you have to do some annoying component counting to establish the index).

When viewing a toplevel post you don't need to do any scroll repositioning, so this PR checks that and disables it for non-replies.

@@ -367,7 +367,7 @@ export const PostThread = observer(function PostThread({
data={posts}
initialNumToRender={posts.length}
maintainVisibleContentPosition={
isNative && view.isFromCache
isNative && view.isFromCache && view.isCachedPostAReply
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wanna leave a comment mentioning the other case is broken? So that it's clear why the condition

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The other case isn't broken in the sense that this is a hackfix. This change is just another way to do it correctly -- if the post we're loading isnt a reply, we don't want to reposition.

@pfrazee pfrazee merged commit 6598fca into main Oct 4, 2023
@pfrazee pfrazee deleted the post-positioning branch October 4, 2023 02:55
estrattonbailey added a commit that referenced this pull request Oct 9, 2023
…e-post-tags-into-app

* origin/main: (40 commits)
  1.52
  README: tweaks to high-level context (#1625)
  Fix stuck lightbox header after double tap (#1627)
  Fix: add padding to the spinner bottom while loading threads (#1626)
  Rewrite Android lightbox (#1624)
  Dont trim before posting (close #1621) (#1622)
  Only listen to back button on android (#1623)
  Improve typeahead search with inclusion of followed users (temporary solution) (#1612)
  Slightly smaller highlighted post text (#1608)
  Pull upstream bugfixes to bottom-sheet (#1606)
  Fix animations and gestures getting reset on state updates in the lightbox (#1618)
  Remove unused lightbox options (#1616)
  Profile UI tweaks (#1607)
  Fix invite codes flash on desktop, use loading placeholder (#1591)
  Update to [email protected] (#1599)
  Fixed a typo on the onboarding recommended screen (#1604)
  Onboarding & feed fixes (#1602)
  Improve time to content in the search page (#1603)
  Fix a potential reference error in bottombarweb (#1600)
  Fix: only use scroll-positioning control on thread when looking at replies (#1587)
  ...
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