Skip to content

Add heic image support#577

Merged
skjnldsv merged 2 commits intomasterfrom
heic
Sep 16, 2020
Merged

Add heic image support#577
skjnldsv merged 2 commits intomasterfrom
heic

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

skjnldsv commented Aug 24, 2020

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

MorrisJobke commented Sep 15, 2020

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

@skjnldsv
Copy link
Member

skjnldsv commented Sep 15, 2020

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

Browser console logs?

@MorrisJobke
Copy link
Member

MorrisJobke commented Sep 15, 2020

Browser console logs?

Nothing suspicious. :/

@skjnldsv
Copy link
Member

skjnldsv commented Sep 15, 2020

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

@MorrisJobke
Copy link
Member

MorrisJobke commented Sep 15, 2020

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

@MorrisJobke
Copy link
Member

MorrisJobke commented Sep 15, 2020

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

skjnldsv commented Sep 15, 2020

@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 commented Sep 16, 2020

@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

skjnldsv commented Sep 16, 2020

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

MorrisJobke commented Sep 16, 2020

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

MorrisJobke commented Sep 16, 2020

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

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@skjnldsv skjnldsv skjnldsv 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