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.

Sign up for GitHub

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

Jump to bottom

Image previews are detached from dialog container #1221

Open
Antreesy opened this issue Feb 1, 2024 · 4 comments
Open

Image previews are detached from dialog container #1221

Antreesy opened this issue Feb 1, 2024 · 4 comments
Labels
bug Something isn't working file picker

Comments

@Antreesy
Copy link

Antreesy commented Feb 1, 2024

To reproduce:

  1. Open Files
  2. Try to move/copy something
  3. Navigate with FilePicker to the folder with many directories
  4. See <img /> elements appended to <body /> flowing on the screen
Screencast.from.01.02.2024.18.09.50.webm

Or:

  1. Open Talk
  2. Try to share something from Nextcloud to conversation
  3. Navigate with FilePicker to the folder with many directories / files
  4. See <img /> elements appended to <body /> flowing on the screen
Screencast.from.01.02.2024.18.13.04.webm
@Antreesy Antreesy added bug Something isn't working file picker labels Feb 1, 2024
@Antreesy
Copy link
Author

Antreesy commented Feb 1, 2024

Could it come from this changes?

nextcloud-dialogs/lib/components/FilePicker/FilePreview.vue

Lines 34 to 38 in 15c0215

const loader = document.createElement('img')
loader.src = previewURL.value.href
loader.onerror = () => loader.remove()
loader.onload = () => { canLoadPreview.value = true; loader.remove() }
document.body.appendChild(loader)

cc @susnux

@susnux
Copy link
Contributor

susnux commented Feb 1, 2024

Yes it is expected to be loaded to the body, but it should not be visible to the user.

@susnux
Copy link
Contributor

susnux commented Feb 1, 2024 •
edited

It adds the file to body to check if loading works and let the browser cache the image.
Otherwise it is impossible to decide if a css background-image works (if not the default MIME fallback icon is shown).

It should not cause any issues, do you experience any issues?

@Antreesy
Copy link
Author

Antreesy commented Feb 1, 2024 •
edited

It adds the file to body to check if loading works and let the browser cache the image.

Can't find the source which says that appending is necessary. Browser should be able to do it in background, when you create img element (or use new Image())

It should not cause any issues, do you experience any issues?

Yes, same floating previews, as on provided screencasts, happen in 28 production (see ap header) . Also freezing the browser, with big amount of img tags appended.

It adds the file to body to check if loading works and let the browser cache the image.

Can we add the class to make it visually hidden, at least?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working file picker
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@susnux @Antreesy