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

Allow slideshow with PDFs #620

Closed
wants to merge 1 commit into from
Closed

Allow slideshow with PDFs #620

wants to merge 1 commit into from

Conversation

skjnldsv
Copy link
Member

Fix #444

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added this to the Nextcloud 25 milestone Jun 29, 2022
@skjnldsv skjnldsv requested a review from a team June 29, 2022 08:21
@skjnldsv skjnldsv self-assigned this Jun 29, 2022
@skjnldsv skjnldsv requested review from PVince81, artonge, Pytal and jancborchardt and removed request for a team June 29, 2022 08:21
@szaimen
Copy link
Collaborator

szaimen commented Jun 29, 2022

Hm.. I am not sure about this as it will now overlay important controls:

Desktop Mobile
image image

Copy link
Collaborator

@szaimen szaimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking. See my comment above

@skjnldsv
Copy link
Member Author

blocking. See my comment above

Hum, no idea how to solve that. Jan approved adding the slideshow

@szaimen
Copy link
Collaborator

szaimen commented Jun 29, 2022

Hum, no idea how to solve that. Jan approved adding the slideshow

yeah, but he was probably not aware about the conflict with these buttons...

@skjnldsv
Copy link
Member Author

@jancborchardt @nimishavijay what do you reckon ? :)

@artonge
Copy link
Contributor

artonge commented Jun 29, 2022

Nice catch @szaimen, I would put the navigation behind the menu. I just checked and that's what the Gnome image viewer is doing:

Closed Open
Screenshot from 2022-06-29 11-23-33 Screenshot from 2022-06-29 11-23-20

@nimishavijay
Copy link
Member

Suggestion by @artonge sounds good to me 🚀

@szaimen
Copy link
Collaborator

szaimen commented Jun 29, 2022

Nice catch @szaimen, I would put the navigation behind the menu. I just checked and that's what the Gnome image viewer is doing:

the only problem that I see with that is that the pdfviewer is loaded in an iframe and I am not sure if it is possible to inject the overlay buttons into the iframe...

cc @skjnldsv

@skjnldsv
Copy link
Member Author

the only problem that I see with that is that the pdfviewer is loaded in an iframe and I am not sure if it is possible to inject the overlay buttons into the iframe...

Nope, indeed. We'd have to think differently :(
Maybe reduce the arrow's height and hide them when the menu is opened 🤔

@szaimen
Copy link
Collaborator

szaimen commented Jun 29, 2022

hide them when the menu is opened 🤔

this sounds good to me 👍

@gpz1100
Copy link

gpz1100 commented Aug 12, 2022

How about just lowering those bottoms, and/or implementing left/right keys on devices with keyboards? There's already a handler for the escape key, why not monitor for L/R too?

Actually, for esc, one needs to click on the top bar (where the X is ) first before escape works.

@gpz1100
Copy link

gpz1100 commented Sep 24, 2022

Has some form of this enhancement made it into the 25.xx beta? I installed 25.0.0 RC1 into a vm but don't see any change in pdf navigation. Thank you

@skjnldsv
Copy link
Member Author

@gpz1100 no, this is not merged, see assigned 26 milestone

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hide them when the menu is opened

For reference, this solution sounds good to me as well. If we can get that event from the PDF viewer we can go ahead with that solution. :)

@skjnldsv skjnldsv modified the milestones: Nextcloud 26, Nextcloud 27 Feb 23, 2023
@skjnldsv skjnldsv modified the milestones: Nextcloud 27, Nextcloud 28 May 3, 2023
@skjnldsv skjnldsv marked this pull request as draft May 3, 2023 08:52
@Io-Qo
Copy link

Io-Qo commented Jul 16, 2023

I think this is a very important feature and should be added soon as navigating through files should be easy within Nextcloud. I reckon the navigation should be added to the sidebar rather than through the slideshow feature, as it would circumvent all problems pointed out by @szaimen and would be an universal approach to all files, not just PDFs etc. Navigation buttons (previous/next file) in the sidebar would make more sense to me.

@blizzz blizzz added this to the Nextcloud 29 milestone Nov 23, 2023
@skjnldsv skjnldsv closed this May 28, 2024
@szaimen szaimen deleted the feat/filelist-fetch branch May 28, 2024 13:05
@gpz1100
Copy link

gpz1100 commented May 29, 2024

I'm confused, is this feature currently offered or will be offered or is being discarded entirely?

@skjnldsv
Copy link
Member Author

@gpz1100 Unfortunately seems the requests for this are quite low, if anyone wants to take over, feel free 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible to utilize slideshow controls from nextcloud/viewer?
9 participants