Lbry-desktop: Toggle fullscreen when pressing `f`

Created on 3 Jan 2019  路  7Comments  路  Source: lbryio/lbry-desktop

The Issue

Pressing f while watching a video doesn't toggle fullscreen.

Steps to Reproduce

  1. Launch a video.
  2. Press f.
  3. Press f again, nothing happened.

Expected Behaviour

Fullscreen should be toggled.

Actual Behaviour

Nothing happens instead.

Suggested Solutions

Implement toggling fullscreen when pressing f.

System Configuration

  • LBRY Daemon version: 0.30.1
  • LBRY App version: 0.26.1
  • LBRY Installation ID: 9ZfMD7MUia6AAvrTDx1MzxGpsEBxUcK3mSxou2VgCFrLSYWwyNavkCTrPBpo5GoTbK
  • Operating system: Linux (Linux-4.18.0-13-lowlatency-x86_64-with-debian-buster-sid)

Anything Else

Screenshots

Internal Use

Acceptance Criteria

  1. If not in fullscreen should go into fullscreen when pressing f.
  2. If already in fullscreen, should go into window screen when pressing f.

Definition of Done

  • [ ] Tested against acceptance criteria
  • [ ] Tested against the assumptions of the user story
  • [ ] The project builds without errors
  • [ ] Unit tests are written and passing
  • [ ] Tests on devices/browsers listed in the issue have passed
  • [ ] QA performed & issues resolved
  • [ ] Refactoring completed
  • [ ] Any configuration or build changes documented
  • [ ] Documentation updated
  • [ ] Peer Code Review performed
ux viewer help wanted 2 new feature

Most helpful comment

I could take a shot, though it might take me a while given how busy I am.

Question: would it be OK if it is hardwired?

Appreciation is always welcome :-)

All 7 comments

Thanks for opening this issue! I agree, this would be a nice shortcut to have! Do you want to take a shot at implementing it?

Can we send you some appreciation for opening the issue?

I could take a shot, though it might take me a while given how busy I am.

Question: would it be OK if it is hardwired?

Appreciation is always welcome :-)

@ngeiswei hmm, not sure what you mean by hardwired. Maybe this previous PR to add a different shortcut would help: https://github.com/lbryio/lbry-desktop/pull/2003

By hardwired I mean that the shortcut, here f, is specified in the code rather than in a config file that is accessible to the user.

BTW, I had a look at the code and I think I can handle it (especially if hardwired), just need to gather free time for it.

Hey @ngeiswei

This is a great idea! This shouldn't be accessible to the user, but should just happen in the background like youtube does.

f should maximize the screen, but also minimize it.

Since we already have this functionality for double-click, it should be pretty straight forward to add.
You could add the event listener right below this line: https://github.com/lbryio/lbry-desktop/blob/master/src/renderer/component/fileViewer/internal/player.jsx#L110

We just need to make sure we remove it when the component is unmounted. And make sure we don't do anything if the input bar has focus (so we don't fullscreen when someone types an f into the search bar). You can check the focused value in the search reducer.

Hi @seanyesmunt, yeah, player.jsx L110 is exactly where I had my eyes on.

We just need to make sure we remove it when the component is unmounted

I don't fully understand that but I guess it will become clear as I dwell deeper into the code.

Thanks for the other tips.

@ngeiswei No problem.

I just meant that we need to remove the event listener when the react component leaves the page. We don't care if a user pressed f on the settings page. We do something similar here:
https://github.com/lbryio/lbry-desktop/blob/master/src/renderer/component/wunderbar/view.jsx#L39-L41

Was this page helpful?
0 / 5 - 0 ratings

Related issues

LBRYansUnited picture LBRYansUnited  路  4Comments

philipzae picture philipzae  路  3Comments

ElectronEsq picture ElectronEsq  路  3Comments

btzr-io picture btzr-io  路  5Comments

eggplantbren picture eggplantbren  路  4Comments