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

[+] FullScreen in SlideShow #454

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0xb0ba
Copy link
Contributor

@0xb0ba 0xb0ba commented Oct 27, 2015

mobile device dont have "F" key for switch to fullscreen.
switch to fullscreen on start slideshow.

@oparoz
Copy link
Contributor

oparoz commented Oct 27, 2015

Thanks for the PR.
This is a partial solution for #252

@deMattin @jancborchardt @jospoortvliet @Tramour @pascalBokBok

@jancborchardt
Copy link
Member

I’d say having a button to trigger fullscreen is better than this, since going fullscreen on viewing any image really messes with people’s flow. It hides the browser and system controls and is quite invasive, no?

What do you think @owncloud/designers?

@jospoortvliet
Copy link

If you play a slideshow, you want to watch pictures. The rest is just distraction. Once the slideshow is over, you close it. What use is watching a slideshow in a tiny window?

I vote for doing this automatically rather than adding a new button.

Note that this was discussed on #252

@oparoz
Copy link
Contributor

oparoz commented Nov 2, 2015

@jancborchardt, @jospoortvliet is right, this only happens when playing the slideshow, so shouldn't really break any workflow.

@jancborchardt
Copy link
Member

Ah sorry, I was under the impression this would already trigger when opening the image. Great then! 👍

(I’m a bit out of the loop cause of the vacation and subsequent confusion induced by New Zealand. ;D)

@jospoortvliet
Copy link

I can't judge the code, but @oparoz can and if it's up to snuff, please merge ;-)

@jancborchardt
Copy link
Member

Would be cool if some more people can pull the code of this PR and actually try it out to test. :)

@deMattin @Tramour @pascalBokBok cause you asked for this in the issue.

@oparoz
Copy link
Contributor

oparoz commented Nov 2, 2015

Soon :)

@oparoz
Copy link
Contributor

oparoz commented Nov 3, 2015

OK, so the code is working, but there are a few little annoyances:

  • on desktop, if you leave full screen then you have to stop autoplay and start it again to go full screen since there is no button. It's a bit disorientating
  • if you press escape, as indicated by the browser, you leave both the full screen mode and the slideshow
  • same thing if you press the close button. It was designed that way, but is a bit surprising after you're automatically thrown into full screen mode

So this could be merged as-is, but I'd like more people to give it a go and tell me what they think.

@0xb0ba
Copy link
Contributor Author

0xb0ba commented Nov 3, 2015

on desktop, if you leave full screen then you have to stop autoplay and start it again to go full screen since there is no button. It's a bit disorientating

I could not repeat. in any sequence buttons are in the right place.

if you press escape, as indicated by the browser, you leave both the full screen mode and the slideshow

This behavior is inherent in the handler buttons. it's easy to fix. but see below ...

same thing if you press the close button. It was designed that way, but is a bit surprising after you're automatically thrown into full screen mode

imho it is logical. "play" and "pause" switched on and off a slideshow/fuulscreen. "escape" or "close" out of preview.

@oparoz
Copy link
Contributor

oparoz commented Nov 3, 2015

I could not repeat. in any sequence buttons are in the right place.

Yes, buttons are in the right place. What I mean is that if you start the slideshow and then press on "exit the slideshow", then the slideshow continues, but you have no way of going back to full screen without stopping the slideshow and starting it again.
It may not be a real issue as we don't always do things the "normal" way when testing stuff, but I still found it a bit strange, so I'd like to get more feedback. In theory, that's the right way to do things if we don't want to introduce a button, but it may not be what users expect.

imho it is logical. "play" and "pause" switched on and off a slideshow/fuulscreen. "escape" or "close" out of preview.

The problem is that the browser is telling users that "esc" should be used to exit fullscreen, but then users will be taken back to the photowall instead of the regular slideshow. In any case, I think we should change the shortcut in another PR so that users can exit the slideshow with "x".

@setnes
Copy link
Contributor

setnes commented Nov 4, 2015

I have very mixed feelings about using the play/pause button to enter and exit full screen. Although it is logical you might sometimes want to do that, you might not always want to do that. It is then awkward to start an automatic slideshow and decide you didn't want to go fullscreen.

This issue started with a concern about mobile devices. Part of the mobile issue is really caused by a problem with the scroll bars in core ownCloud. I am considering logging an issue there. Even if you are in the files view, the browser bar never hides because the page never looks any bigger than the device it is displayed on. If you click on a photo and the browser bar is already hidden, the slideshow will fill the whole screen without prompting the user to "go fullscreen". It's far less intrusive the way it was before the scroll bar changes.

Also, I don't actually find the play button useful at all on mobile devices. Automatically advancing through photos gets in the way of your natural desire to advance to the next photo when you're good and ready. In fact, it's very similar if you have a keyboard in front of you. It's rare to want to look at each photo for the exact same amount of time. You inevitably end up using the arrow keys while the slideshow is running and then swear when you realize you forgot the timer was going and it flips to the next photo.

Maybe this is a little drastic, but I'd much rather add a full screen button and remove the play/pause button all together. I understand that probably won't happen, but I'm trying to make a point. If I was to rank the importance of the two buttons I'd put fullscreen before play/pause.

@setnes
Copy link
Contributor

setnes commented Nov 4, 2015

Also... as far as this pull request.

Click on the play button and go full screen. Then use the "F" key to come out of full screen. The slide show continues, but it can take multiple button clicks on play/pause to get it to stop. It doesn't seem consistent.

@0xb0ba
Copy link
Contributor Author

0xb0ba commented Nov 4, 2015

Then use the "F" key to come out of full screen.

"F" - exit from fuulscreen

The slide show continues

yes, "exit" or "pause" of the slideshow was not pressed

but it can take multiple button clicks on play/pause to get it to stop

no, one press "exit" for leave preview or one press "pause" for stop slideshow.

@oparoz
Copy link
Contributor

oparoz commented Nov 4, 2015

it can take multiple button clicks on play/pause to get it to stop. It doesn't seem consistent.

I wasn't able to reproduce. Which browser?

@jancborchardt
Copy link
Member

Yeah, what @setnes says makes a lot of sense. Automatically advancing photos is a strange thing nowadays, platforms like Flickr, Facebook etc don’t do that at all. Fullscreen is a more important action and should replace that. Additionally, clicking the image should advance to the next image, as discussed elsewhere – this is how most major platforms do it.

@oparoz
Copy link
Contributor

oparoz commented Nov 4, 2015

Yes, the play button will move to the 3 dots. It's still useful if you use Gallery in a real gallery per example and have a display cycling through images.

@0xb0ba
Copy link
Contributor Author

0xb0ba commented Nov 4, 2015

I show a photo to a friend as a slideshow. and when I need to press the pause.

@0xb0ba
Copy link
Contributor Author

0xb0ba commented Nov 4, 2015

and the browser bar is already hidden

this is not happening.

I don't actually find the play button useful at all on mobile devices

most likely you're right

@setnes
Copy link
Contributor

setnes commented Nov 7, 2015

and the browser bar is already hidden

this is not happening.

Correct. This does not happen right now. Sorry to mislead. This is how it used to work. It doesn't appear there is currently a way for the browser bar to auto hide. This is the result of changes in core.

@oparoz oparoz modified the milestones: 9.1-next, 9.0-current Feb 11, 2016
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.

6 participants