search

flarum / issue-archive

0 stars 0 forks source link

Notifications panel scrolling doesn't work on high-resolution displays #120

Open matteocontrini opened 3 years ago

matteocontrini commented 3 years ago

I have a report that the scrolling of the notifications panel is still broken, and it appears to be related to the fact that the screen has a 4K resolution. The problem appears on multiple browsers. I'm told that resizing the window to make it smaller fixes the issue.

Video follows.

https://user-images.githubusercontent.com/2164763/112059503-348f7100-8b5c-11eb-9f22-578f786e175f.mp4

davwheat commented 3 years ago

I think it'd be a good idea to rewrite the drop-down/page to remove this issue once and for all.

My suggestion would be to load an amount of notifications based on the height of the viewport initially, or to limit the dropdown height with CSS.

askvortsov1 commented 3 years ago

This particular issue seems to happen because notification list height is fixed at 70vh. Since the callback for checking whether we're at the bottom of the page and loading more happens on scroll, if the entire first page of content fits in those 70vh, there won't be a scrollbar, so there will never be scrolling, so nothing new will be loaded.

davwheat commented 3 years ago

@askvortsov1 We could force the inner container to be calc(70vh + 1px) as a workaround 🤔

askvortsov1 commented 3 years ago

@askvortsov1 We could force the inner container to be calc(70vh + 1px) as a workaround

But then there'd always be a scrollbar, even when unnecessary.

davwheat commented 3 years ago

@askvortsov1 We could check the links of the notifications XHR response. If links.next exists, then there are more notifications, otherwise there aren't.

This could be used to force the inner container to be 70vh + 1px if more notifications are available.

dsevillamartin commented 3 years ago

We could check the links of the notifications XHR response. If links.next exists, then there are more notifications, otherwise there aren't.

This can be done once flarum/framework#2781 is merged, as it includes hasNext methods and stuff.