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

Scrolling of already loaded photos is not smooth, previews get unloaded #232

Closed
jancborchardt opened this issue Mar 12, 2020 · 15 comments · Fixed by #488
Closed

Scrolling of already loaded photos is not smooth, previews get unloaded #232

jancborchardt opened this issue Mar 12, 2020 · 15 comments · Fixed by #488
Labels
1. to develop Accepted and waiting to be taken care of bug Something isn't working high High priority

Comments

@jancborchardt
Copy link
Member

Describe the bug
Scrolling up and down through the list is not smooth as previews of previously loaded pictures get unloaded. The image filetype icon is shown and the preview is loaded again, leading to flickering.

To Reproduce
Steps to reproduce the behavior:

  1. Go to any view
  2. Scroll down
  3. Scroll back up again, and down, and up

Expected behavior
Scrolling is nice and smooth, you ideally never see any picture preview loading – but especially not for ones which you already scrolled past and are scrolling back up to.

Screenshots
Here’s a gif:
Photos scrolling jank

Using latest Photos app master.

@jancborchardt jancborchardt added bug Something isn't working high High priority 1. to develop Accepted and waiting to be taken care of labels Mar 12, 2020
@skjnldsv
Copy link
Member

Do you have the dev tools opened?

@jancborchardt
Copy link
Member Author

Do you have the dev tools opened?

It happens with dev tools closed and opened, either way. What’s your question? ;)

@skjnldsv
Copy link
Member

It happens with dev tools closed and opened, either way. What’s your question? ;)

if dev tools opened = cache disabled = force fetch all photos every time ;)

@Mikescops
Copy link
Member

@jancborchardt can you confirm that it is no more the case with the latest master?

@skjnldsv
Copy link
Member

skjnldsv commented Oct 15, 2020

Btw, nice mustachio @jancborchardt 😁 😍
Capture d’écran_2020-10-15_13-58-40

@jancborchardt
Copy link
Member Author

@jancborchardt can you confirm that it is no more the case with the latest master?

@Mikescops no, seems this still exists – here’s another gif of me scrolling up and down quickly:
Peek 2020-10-15 17-34

@Mikescops
Copy link
Member

@skjnldsv I bet this is our caching problem with N21

@skjnldsv
Copy link
Member

@Mikescops seems so yeah

@Mikescops
Copy link
Member

@skjnldsv to be honest i have removed the @loaded again and this issue is nearly unvisible, but I think we will need the worker cache as you suggested.

@skjnldsv
Copy link
Member

skjnldsv commented Oct 15, 2020

@Mikescops
Copy link
Member

I think I will use Google Workbox, seems to be the most reliable implementation and has webpack support.

@skjnldsv
Copy link
Member

Nice!
Remember the cache buster is the etag :)

@ghost
Copy link

ghost commented Oct 16, 2020

Yup i have the same issue when loading photos as @jancborchardt

@ntimo
Copy link

ntimo commented Oct 23, 2020

Is there a change this gets backported to NC20?

@Mikescops
Copy link
Member

Nope, this is for NC21 only due to the new grid we implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug Something isn't working high High priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants