#1

If I navigate to a topic with only 1 unread post, then the scroll position ends up about halfway down the post.

I’ve noticed this on various topics on Meta, and other forums, so I don’t think it’s topic specific. I have only noticed this on mobile (iPhone 7, latest iOS)

Here’s a video of the behaviour:

9 Likes
#2

I believe I can repro this on mobile for all topics @tgxworld — every topic I enter I am seeing the bottom half of latest post :frowning:

3 Likes
#3

Does anyone know if it happens on android too? Curious if this is a jQuery related regression. Note not seeing any issues when entering on last post on my iPhone, so somehow this is only for 1 unread.

#4

I have noticed something similar to this.
If you don’t read all the new posts in a topic, when you visit it again it will be wear you last read all the post up to last time.
For example-
One day I read all 50 posts in a topic
10 more posts are created in that topic
I only read 8 of those new posts
I go back to the 50 post mark

#5

I have an Android phone. I can test it for you.

#6

Yes it does. At least on the Samsung Galaxy S7
Google Chrome 65

#9

OK I am going to have to hand this off to @eviltrout, it seems that a lot of the pain here is coming from scrollTopFor (which is calling offsetCalculator 3 times for some reason - do we know why ?)

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/discourse/lib/offset-calculator.js.es6

It is entirely possible that this is due to some internal changes in jQuery regarding #offset however I can oddly get reasonable results bypassing it altogether and just using this in lock on:

elementTop() {
    const selected = $(this.selector);
    if (selected && selected.offset && selected.offset()) {
      const result = selected.offset().top;
      return (result - $('header').outerHeight()) - 10;
      //return result - Math.round(scrollTopFor(result));
    }
  }

The repro of the issue happens when you make a very narrow window, it trades in percentages of height so being off a few pixels gets amplified big time on narrow displays.

So I guess my big question is, do we need to carry all this fancy or should we simply amend it to always try to place the post highlighted a few pixels below header? I get that there may be some value in displaying 1 previous post IF you can fit current post entirely on-screen, but this does feel somewhat more complex to wrangle.

Note, the regression is likely my fault and in particular jQuery changes, however I think we should revisit the basics here and decide how much fancy we need?

4 Likes
#11

It is super ugly, but it was the only thing that worked consistently when I wrote the code. I had many bugs with it displaying in the wrong place and noticed it worked properly if you tried a few times in a row.

The logic is used in several places, notably in the scrolling post stream to determine what post you’re currently looking at. What worries me if we replace lock-on with a different algorithm, is our eyeline tracking broken now?

Replacing this could cause a lot of regressions, particularly when the header has been customized by CSS to be larger or to contain custom content.

I have a lot on my plate right now, so my vote would be to revert jQuery for now if that’s the cause and revisit when we have more time.

2 Likes
#12

Just to add to this, I’m using a desktop (Windows 10/Edge). Any topic I click on, I’m having this issue with the last post.

Continually have to scroll up to see the post.

#13

I’ve noticed some weirdness with desktop Vivaldi that does not happen with desktop Chrome. When I go to a topic that has a new unread post the “top” is under the header (it is “top” but not offset to allow for the header height) and I need to scroll to see the first bit of the post. It happens here at meta, but not at SitePoint if that helps any.

EDIT
I no sooner posted this, and it is no longer happening. in fact they are a bit low if anything. :confused:

#14

@sam, not sure if this is related, but I’ve found that after entering a topic that has 1 unread reply, using j or k takes me to the bottom of suggested topics instead of to the post I am reading (going to the first suggested topic would be okay too, but the bottom… that’s a bit much)

#15

Just want to register that this is an ongoing issue on the Drowned in Sound boards too on mobile, being flicked to just below the post you’re notified of or to the very bottom of the last post when you open a new topic.

#16

Did you revert this @sam? It’s quite bad the way it is now, so you either need to own the fix, or revert.

#17

This is fixed per:

https://github.com/discourse/discourse/commit/8c1d145f0eaa8539a920c44537c0a64192650e1d

This also removes the 3 times a charm hack so cause I want to see if it is actually needed, can not repro a problem with it.

The issue was actually quite straightforward .height() in the new jQuery returns undefined if you call it on a non element whereas in the past it returned 0, I guess it is more correct now, but we need to be careful.

I also corrected some calculations that incorrectly accounted for title height and normalized keyboard shortcuts and highlight so they behave in the same way.

16 Likes
#18

Awesome, thank you for this … cleaner code FTW

4 Likes
#19

For the record, this is still happening for me. It must not be related to the scroll position though, definitely happens on Desktop, Chrome v65.

1 Like
#20

Yeah I have a repro of this but it is unrelated. Can you open a new topic on this.

3 Likes
#21

It was definitely needed. We should keep a close eye out for regressions in the next couple of weeks just to be sure we didn’t lose something.

3 Likes
Broken pull down to reload in Android Chrome
#22

Mine is fixed! Thank you so much for your hard work! :upside_down_face:

2 Likes
#23

^^^^

Spoke too soon.

I’m right back where I started. Last post is completely out of the screen.