pleroma/pleroma-fe
Watch
2
2
Fork
You've already forked pleroma-fe
 10
Code Issues 351 Pull requests 26 Projects Releases 5 Packages 1 Wiki Activity

Timeline virtual scrolling #2310

Closed
shpuld wants to merge 49 commits from gitlab-mr-iid-1043 into develop
pull from: gitlab-mr-iid-1043
merge into: pleroma:develop
Conversation 54 Commits 49 Files changed 21 +203 -38
shpuld commented 2020-01-15 16:44:34 +00:00
Member
Copy link

This MR optimizes the amount of stuff in dom/v-dom a lot by simplifying the statuses that are not in view. This is most notable when observing RAM use while scrolling deep down and loading lots of statuses on the timeline that way. Not much of an impact you'll feel on high performance devices that run the application smooth enough already.

There aren't many notable downsides to the way it's done, as each Conversation/Status component is kept alive when they're hidden and they get to keep their state untouched. The biggest ones are:

  • Not being able to use ctrl-f on the full page because a lot of the text outside of view is hidden
  • When resizing, the calculated heights for hidden elements is going to be wrong and scrolling up the first time after that will be jerky.

There is an option to turn the feature off for those who encounter problems.

EDIT: after extended on my instance I've noticed some parts regarding the perf that I can improve

EDIT2: I haven't really been running into any issues with the latest version of this. Only positives making the FE more responsive especially on mobile.

This MR optimizes the amount of stuff in dom/v-dom a lot by simplifying the statuses that are not in view. This is most notable when observing RAM use while scrolling deep down and loading lots of statuses on the timeline that way. Not much of an impact you'll feel on high performance devices that run the application smooth enough already. There aren't many notable downsides to the way it's done, as each Conversation/Status component is kept alive when they're hidden and they get to keep their state untouched. The biggest ones are: - Not being able to use ctrl-f on the full page because a lot of the text outside of view is hidden - When resizing, the calculated heights for hidden elements is going to be wrong and scrolling up the first time after that will be jerky. There is an option to turn the feature off for those who encounter problems. EDIT: after extended on my instance I've noticed some parts regarding the perf that I can improve EDIT2: I haven't really been running into any issues with the latest version of this. Only positives making the FE more responsive especially on mobile.
hj commented 2020-01-15 17:04:19 +00:00
Member
Copy link

w

w
shpuld commented 2020-01-15 17:22:04 +00:00
Author
Member
Copy link

I don't know how I managed to do that w

I don't know how I managed to do that w
lambadalambda commented 2020-06-21 12:25:50 +00:00
Owner
Copy link

Is there anything to help with to get this into a mergeable state? Every time i tried it it was such a big improvement

Is there anything to help with to get this into a mergeable state? Every time i tried it it was such a big improvement
shpuld commented 2020-06-21 19:51:44 +00:00
Author
Member
Copy link

it's better now as I've tested a more fixed version, but I still need to investigate some browser specific issues, and maybe figure out how to make scrolling get fucky when you use the back button

it's better now as I've tested a more fixed version, but I still need to investigate some browser specific issues, and maybe figure out how to make scrolling get fucky when you use the back button
lambadalambda commented 2020-08-31 14:54:08 +00:00
Owner
Copy link

Going to a post directly does not display anything: https://shpposter.club/notice/9ygQmNndbd6Io8zrge

Going to a post directly does not display anything: https://shpposter.club/notice/9ygQmNndbd6Io8zrge
shpuld commented 2020-08-31 16:11:07 +00:00
Author
Member
Copy link

"this.status is undefined"

"this.status is undefined"
hj commented 2020-08-31 16:26:24 +00:00
Member
Copy link

i've seen something similar on regular (pre-2.1) builds too, not sure if it was same error tho

i've seen something similar on regular (pre-2.1) builds too, not sure if it was same error tho
shpuld commented 2020-08-31 16:59:21 +00:00
Author
Member
Copy link

it's not exact same, I see why it happens here and it's easy to fix, just need to do it

it's not exact same, I see why it happens here and it's easy to fix, just need to do it
shpuld commented 2020-09-01 06:47:11 +00:00
Author
Member
Copy link

fixed this last night (not yet deployed on shpc)

fixed this last night (not yet deployed on shpc)
lambadalambda commented 2020-09-01 09:38:20 +00:00
Owner
Copy link

can confirm it works again

can confirm it works again
lambadalambda commented 2020-09-03 08:56:20 +00:00
Owner
Copy link

I've been using this version on lain.com for the last few days and haven't encountered any issues anymore. Ready to go in, from my point of view.

I've been using this version on lain.com for the last few days and haven't encountered any issues anymore. Ready to go in, from my point of view.
shpuld commented 2020-09-03 09:02:40 +00:00
Author
Member
Copy link

just need actual code review then cc @hj

just need actual code review then cc @hj
hj commented 2020-09-03 14:39:44 +00:00
Member
Copy link

yeah, hopefully today/tomorrow

yeah, hopefully today/tomorrow
hj commented 2020-09-04 00:34:04 +00:00
Member
Copy link

Videos always loop initially even if they have sound and setting is set to NOT loop the videos with sound.

Videos always loop initially even if they have sound and setting is set to NOT loop the videos with sound.
hj commented 2020-09-04 00:37:40 +00:00
Member
Copy link

Scrolling sometimes feels a bit laggy at time, but switching timelines feels MUCH better, good job!

Scrolling sometimes feels a bit laggy at time, but switching timelines feels MUCH better, good job!
hj commented 2020-09-04 00:38:12 +00:00
Member
Copy link

i'd say there probably should be some throttle/debounce to when posts start load when scrolling into void area

i'd say there probably should be some throttle/debounce to when posts start load when scrolling into void area
hj commented 2020-09-04 00:39:14 +00:00
Member
Copy link

there's no setting in UI but setting exists in the code

there's no setting in UI but setting exists in the code
hj commented 2020-09-04 00:45:42 +00:00
Member
Copy link

This is intended, classes that begin with - are modifiers, RSCSS convention.

    :class="[{ '-focused': isFocused }, { '-conversation': inlineExpanded }]"
This is intended, classes that begin with `-` are modifiers, RSCSS convention. ```suggestion:-0+0 :class="[{ '-focused': isFocused }, { '-conversation': inlineExpanded }]" ```
hj commented 2020-09-04 00:45:43 +00:00
Member
Copy link

ew, flush after yourself my dude
e37e7e8154cc01f6c8a9d355af5de58a7a21583abdbb9287baf8486bb5dbc47d

ew, flush after yourself my dude ![e37e7e8154cc01f6c8a9d355af5de58a7a21583abdbb9287baf8486bb5dbc47d](/attachments/cd583046-3bf5-4cf8-8ade-a0fde3cbd9f6) ```suggestion:-0+0 ```
e37e7e8154cc01f6c8a9d355af5de58a7a21583abdbb9287baf8486bb5dbc47d.mp4
1.2 MiB
hj commented 2020-09-04 00:45:43 +00:00
Member
Copy link

this is the reason why videos always loop, and below too.

this is the reason why videos always loop, and below too.
hj commented 2020-09-04 00:45:43 +00:00
Member
Copy link

probably this.hasAudio !== undefined?

probably `this.hasAudio !== undefined`?
hj commented 2020-09-04 00:45:55 +00:00
Member
Copy link

at least i think

at least i think
shpuld commented 2020-09-04 08:46:46 +00:00
Author
Member
Copy link

it already has it throttled with 200ms, much more results in more visual glitching

it already has it throttled with 200ms, much more results in more visual glitching
shpuld commented 2020-09-04 08:47:02 +00:00
Author
Member
Copy link

merge casualty, fixing

merge casualty, fixing
shpuld commented 2020-09-04 08:48:04 +00:00
Author
Member
Copy link

it was something missing, had to manually change it to get it working at all, I'll look into it more, moving the status-conversation class under Status or something

it was something missing, had to manually change it to get it working at all, I'll look into it more, moving the `status-conversation` class under `Status` or something
shpuld commented 2020-09-04 08:52:11 +00:00
Author
Member
Copy link

uhh, looks like the whole thing is a CSS refactor casualty, there's not race of "-conversation" classes, I wonder what has broken...

uhh, looks like the whole thing is a CSS refactor casualty, there's not race of "-conversation" classes, I wonder what has broken...
shpuld commented 2020-09-04 09:06:34 +00:00
Author
Member
Copy link

the casualty is non-inline conversations having the red line, might fix that here.

the casualty is non-inline conversations having the red line, might fix that here.
shpuld commented 2020-09-04 09:12:11 +00:00
Author
Member
Copy link

there's major changes here to make it make more sense, note the change in name as well

it worked correct with all settings when I made it many many months ago, I'll check again what's up

there's major changes here to make it make more sense, note the change in name as well it worked correct with all settings when I made it many many months ago, I'll check again what's up
shpuld commented 2020-09-04 10:25:29 +00:00
Author
Member
Copy link

it cant be anything but true or false as it's always explicitly set to one of the two

it cant be anything but `true` or `false` as it's always explicitly set to one of the two
shpuld commented 2020-09-04 10:35:37 +00:00
Author
Member
Copy link

oh yeah now I remember some details about it

firefox is BROKEN AS HELL, it's not calling play or playing events. works fine in chromium

oh yeah now I remember some details about it firefox is BROKEN AS HELL, it's not calling play or playing events. works fine in chromium
shpuld commented 2020-09-04 10:38:49 +00:00
Author
Member
Copy link

never mind, it's being called, I was doing something wrong and it wasn't using my logged code

never mind, it's being called, I was doing something wrong and it wasn't using my logged code
hj commented 2020-09-04 10:39:20 +00:00
Member
Copy link

yeah but it seems like intention is to not run any code if hasAudio is set, this code will not execute if hasAudio is set to false

yeah but it seems like intention is to not run any code if **hasAudio is set**, this code will not execute if hasAudio is set to `false`
shpuld commented 2020-09-04 10:41:12 +00:00
Author
Member
Copy link

Ok, I tested all settings combinations with both sound/no-sound videos, everything works exactly like it should?

how do you get it to break on yours?

Ok, I tested all settings combinations with both sound/no-sound videos, everything works exactly like it should? how do you get it to break on yours?
shpuld commented 2020-09-04 10:42:29 +00:00
Author
Member
Copy link

I see what you mean, I'll fix it

I see what you mean, I'll fix it
hj commented 2020-09-04 10:43:28 +00:00
Member
Copy link

When post initially loaded video will ALWAYS loop. Switching relevant settings on and off fixes existing videos. Any new videos that load will still loop.

At least how it was for me yesterday.

When post initially loaded video will ALWAYS loop. Switching relevant settings on and off fixes existing videos. Any new videos that load will still loop. At least how it was for me yesterday.
shpuld commented 2020-09-04 10:45:29 +00:00
Author
Member
Copy link

actually no, it is how it should be, hasAudio is initialized to false, on first play we check if there's audio, if there is set it to true. A video can never lose its audio, so there's no point ever changing it once its set to true once.

EDIT: last true was false, fixed

actually no, it is how it should be, `hasAudio` is initialized to `false`, on first play we check if there's audio, if there is set it to `true`. A video can never lose its audio, so there's no point ever changing it once its set to `true` once. EDIT: last true was false, fixed
shpuld commented 2020-09-04 10:52:43 +00:00
Author
Member
Copy link

works for me on both firefox and chromium, tried refreshing and playing videos immediately as they show up

works for me on both firefox and chromium, tried refreshing and playing videos immediately as they show up
shpuld commented 2020-09-04 10:54:58 +00:00
Author
Member
Copy link

Added a comment about it to help reduce further confusion.

Added a comment about it to help reduce further confusion.
hj commented 2020-09-04 11:16:59 +00:00
Member
Copy link

https://shigusegubu.club/notice/9ynXaOsI10qwHM78M4 this fails and always loops.
https://shigusegubu.club/notice/9ynY1pm46xZvWdHgJM this works correctly.

Dunno why, but both start with loop="loop" but one that works has that attribute removed while the one that broken becomes loop=""

smells like a vue bug

https://shigusegubu.club/notice/9ynXaOsI10qwHM78M4 this fails and always loops. https://shigusegubu.club/notice/9ynY1pm46xZvWdHgJM this works correctly. Dunno why, but both start with `loop="loop"` but one that works has that attribute removed while the one that broken becomes `loop=""` smells like a vue bug
hj commented 2020-09-04 11:18:14 +00:00
Member
Copy link

consolelog-driven development has shown that in both cases they both properly detect audio

consolelog-driven development has shown that in both cases they both properly detect audio
hj commented 2020-09-04 11:26:00 +00:00
Member
Copy link

only happens on firefox (on that video at least) and not in chrome

only happens on firefox (on that video at least) and not in chrome
hj commented 2020-09-04 11:31:12 +00:00
Member
Copy link

Workaround to this is to set hasAudio to true initially and then reset it to false if no audio track is detected.

Workaround to this is to set `hasAudio` to `true` initially and then reset it to `false` if no audio track is detected.
shpuld commented 2020-09-04 11:52:41 +00:00
Author
Member
Copy link

that's interesting, I'm not against changing the ordering to start with true, but I'll investigate a bit more

that's interesting, I'm not against changing the ordering to start with true, but I'll investigate a bit more
shpuld commented 2020-09-04 11:53:29 +00:00
Author
Member
Copy link

the always looping video doesn't loop for me when I find it on laindotcom, I'll try setting my dev server to sgsgb

the always looping video doesn't loop for me when I find it on laindotcom, I'll try setting my dev server to sgsgb
hj commented 2020-09-04 11:56:56 +00:00
Member
Copy link

might not help either. I literally have no idea why it happens. Might be related to number of elements on page, i.e notifications.

at least it seems like vue is having problems removing the loop properly, so workaround where we add it instead seems reasonable.

man reporting this bug to vue is going to be ASS

might not help either. I literally have no idea why it happens. Might be related to number of elements on page, i.e notifications. at least it seems like vue is having problems removing the `loop` properly, so workaround where we *add* it instead seems reasonable. man reporting this bug to vue is going to be ASS
shpuld commented 2020-09-04 12:32:13 +00:00
Author
Member
Copy link

good point, that might explain why it breaks. I'll change the code and test that it works

good point, that might explain why it breaks. I'll change the code and test that it works
shpuld commented 2020-09-04 12:32:58 +00:00
Author
Member
Copy link

changed, looks like I didn't actually post the previous comment, thanks gitlab's useless review system

changed, looks like I didn't actually post the previous comment, thanks gitlab's useless review system
hj commented 2020-09-22 19:06:30 +00:00
Member
Copy link

I'm gonna dogfood it a bit on sgsgb. Last time it worked so good we can probably just enable it permanently.

I'm gonna dogfood it a bit on sgsgb. Last time it worked so good we can probably just enable it permanently.
hj commented 2020-09-23 22:19:36 +00:00
Member
Copy link

there are some CSS bugs, some of which might be in develop as well. I'd say no need to fix them in this MR, I'll fix them all in one swoop after this is merged.

there are some CSS bugs, some of which might be in develop as well. I'd say no need to fix them in this MR, I'll fix them all in one swoop after this is merged.
hj commented 2020-09-23 22:21:30 +00:00
Member
Copy link
  • Red stripe is missing in conversations
  • Avatar shadow is screwed [DEVELOP]
  • Emoji react popup is wider than putin
  • GIF indicator doesn't disappear on hover [DEVELOP]
* Red stripe is missing in conversations * Avatar shadow is screwed [DEVELOP] * Emoji react popup is wider than putin * GIF indicator doesn't disappear on hover [DEVELOP]
shpuld commented 2020-09-29 08:53:47 +00:00
Author
Member
Copy link

red stripe could be a theme bug, doesn't happen on my instance

red stripe could be a theme bug, doesn't happen on my instance
shpuld commented 2020-09-29 08:56:10 +00:00
Author
Member
Copy link

and neither does wide react popup..

and neither does wide react popup..
hj commented 2020-09-29 09:56:26 +00:00
Member
Copy link

wide react popup is related to lain's issue #922

either way i'll fix it all myself some time later.

wide react popup is related to lain's issue #922 either way i'll fix it all myself some time later.
lambadalambda commented 2020-09-29 10:17:19 +00:00
Owner
Copy link

Hit that merge button

Hit that merge button

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
BE Request

   
Backlog

   
Better behavior

   
Blocked by deps

   
Bug

   
Code Review

   
Discussion

   
Doing

   
Feature Request / Enhancement

   
In the shining bright future maybe

   
It's complicated

   
MS Edge

   
Mememoon (or other niche browsers)

   
Missing API

   
Missing feature

   
Missing l10n/i18n

   
NL1
Instance Admin Dashboard

  
NL2
Improved Mobile Experience

  
NL4
Improved instance customization and QoL options

  
Need to verify on develop

   
RELEASE BLOCKER

   
Reassign or Close

   
Refactor

   
Regression

   
Safari

   
To Do

   
User story

   
Waiting on godot

   
accessibility
https://git.pleroma.social/pleroma/pleroma-fe/-/snippets/7746

  
better documentation

   
chore

   
confirmed

   
easy ticket

   
incident
Denotes a disruption to IT services and the associated issues require immediate attention

  
l10n update

   
mastoapi

   
mobile

   
needs design

   
needs-info

   
needs-review

   
stupid
No labels
BE Request
Backlog
Better behavior
Blocked by deps
Bug
Code Review
Discussion
Doing
Feature Request / Enhancement
In the shining bright future maybe
It's complicated
MS Edge
Mememoon (or other niche browsers)
Missing API
Missing feature
Missing l10n/i18n
NL1
NL2
NL4
Need to verify on develop
RELEASE BLOCKER
Reassign or Close
Refactor
Regression
Safari
To Do
User story
Waiting on godot
accessibility
better documentation
chore
confirmed
easy ticket
incident
l10n update
mastoapi
mobile
needs design
needs-info
needs-review
stupid
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
pleroma/pleroma-fe!2310
No description provided.