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

Add heic image support #577

Merged
merged 2 commits into from
Sep 16, 2020
Merged

Add heic image support #577

merged 2 commits into from
Sep 16, 2020

Conversation

steiny2k
Copy link
Contributor

@steiny2k steiny2k commented Aug 23, 2020 •
edited
Loading

Replaces PR #485. Rebased to master and that's it.

Signed-off-by: Sebastian Steinmetz 462714+steiny2k@users.noreply.github.com

@steiny2k steiny2k mentioned this pull request Aug 23, 2020
Closed
@cypress
Copy link

cypress bot commented Aug 23, 2020


Test summary

150 • 1 • 0 • 0

Run details
Project viewer
Status Failed
Commit 93ee967 ℹ️
Started Aug 23, 2020 1:58 PM
Ended Aug 23, 2020 2:00 PM
Duration 01:53 💡
OS Linux Ubuntu Linux - 18.04
Browser Electron 78

View run in Cypress Dashboard ➡️

Failures

cypress/integration/sidebar.spec.js ⏯ 1 Failed
1 Open the sidebar from the viewer and open viewer with sidebar already opened > Does not see a loading animation

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@skjnldsv
Copy link
Member

Hey! Please add a test example as well :)
https://github.com/nextcloud/viewer/tree/master/cypress/integration
https://github.com/nextcloud/viewer/tree/master/cypress/fixtures

@skjnldsv skjnldsv added 2. developing Work in progress enhancement New feature or request labels Aug 24, 2020
@MorrisJobke
Copy link
Member

I tried this, but it only says "error while loading image.heic". 😢

@skjnldsv
Copy link
Member

I tried this, but it only says "error while loading image.heic".

Browser console logs?

@MorrisJobke
Copy link
Member

Browser console logs?

Nothing suspicious. :/

@skjnldsv
Copy link
Member

I guess it's just nt supported 🤷 ?
Can you open the file in a new tab?

@MorrisJobke
Copy link
Member

Also this just uses the plain WebDAV endpoint and not the preview one.

@MorrisJobke
Copy link
Member

Bildschirmfoto 2020-09-15 um 19 44 25

Actually there was something in the console :/

@skjnldsv
Copy link
Member

skjnldsv commented Sep 15, 2020 •
edited
Loading

Works for me and uses preview 🤷

Capture d’écran_2020-09-15_22-50-00

https://filesamples.com/samples/image/heic/sample1.heic

@MorrisJobke does your setup have heic previews properly enabled/configured?

@skjnldsv
Copy link
Member

@steiny2k please rebase and add a cypress tests for displaying an heic image.
You can most likely just copy like https://github.com/nextcloud/viewer/blob/master/cypress/integration/image.png.spec.js

@MorrisJobke
Copy link
Member

@MorrisJobke does your setup have heic previews properly enabled/configured?

No. Do I need that? I just use the default configuration. And then it loads the file via WebDAV.

@skjnldsv
Copy link
Member

No. Do I need that? I just use the default configuration. And then it loads the file via WebDAV.

you have a GET request to webdav?

@MorrisJobke
Copy link
Member

you have a GET request to webdav?

Yes, multiple. one to each file of the folder on every slide change. So if there are 3 heic images in a folder and I click on one, then the viewer opens and all files are fetched. On the slideshow change to the next all 3 are fetched again and so on.

@skjnldsv skjnldsv mentioned this pull request Sep 16, 2020
Closed
steiny2k and others added 2 commits September 16, 2020 10:28
@steiny2k @MorrisJobke
Add heic image support
9028deb 
Signed-off-by: Sebastian Steinmetz <462714+steiny2k@users.noreply.github.com>
@MorrisJobke
Add heic integration test
31a3376 
Example file is from https://www.online-convert.com/file-format/heic and under CC-BY http://www.online-convert.com

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

Rebased and added the tests 👍

@MorrisJobke MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 16, 2020
@MorrisJobke MorrisJobke added this to the Nextcloud 20 milestone Sep 16, 2020
@skjnldsv skjnldsv merged commit 6f3b742 into master Sep 16, 2020
@skjnldsv skjnldsv deleted the heic branch September 16, 2020 08:41
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 16, 2020
@MorrisJobke MorrisJobke mentioned this pull request Sep 16, 2020
Merged
5 tasks
@montvid montvid mentioned this pull request Jan 19, 2021
Closed
@szaimen szaimen mentioned this pull request May 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@georgehrke georgehrke Awaiting requested review from georgehrke

@Ivansss Ivansss Awaiting requested review from Ivansss

@marinofaggiana marinofaggiana Awaiting requested review from marinofaggiana

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request
Projects
None yet
Milestone
Nextcloud 20
Development

Successfully merging this pull request may close these issues.
3 participants
@steiny2k @skjnldsv @MorrisJobke