shabados / presenter

Desktop app for presenting the Shabad OS Database on projectors, TVs, and live streams
https://shabados.com
MIT License
19 stars 15 forks source link

feat: add launch in fullscreen on startup toggle #563

Closed AkalUstat closed 4 years ago

AkalUstat commented 4 years ago

when the main window is ready-to-show. Toggling this does not affect while the app is running

Summary of PR

On startup, if the user has the fullscreen setting enabled, the main window will open in fullscreen. Defaults to true

Tests for unexpected behavior

Currently testing: when this is enabled, everything opens in fullscreen (settings, etc). Not sure if this is wanted behavior

Time spent on PR

more time than needs to be known

Linked issues

Fix #18

Reviewers

@bhajneet @Harjot1Singh

bhajneet commented 4 years ago

I know you're still working on this, no worries. Just a small note once you're done with the draft: fullscreen option should be off by default.

AkalUstat commented 4 years ago

Can't get this working as you want (just the display windows)...I spent like 20-25 min actually setting up and now upwards of 4hrs debugging and its not going anywhere.

Harjot1Singh commented 4 years ago

Leave the PR open and mark as draft, we can assist

Harjot1Singh commented 4 years ago

Also, rename the fullscreen key to something a little more descriptive, likefullscreenOnLaunch

AkalUstat commented 4 years ago

still have to push some changes i was playing around with

AkalUstat commented 4 years ago

@Harjot1Singh cant find a solution to this

bhajneet commented 4 years ago

Is this actively being worked on? Please update the linked issue if so

AkalUstat commented 4 years ago

yea im playing around with some items. Turns out this applies everything gets fullscreened only applies to mac (since it fullscreens all windows of an app) -> might be a workaround for that. Windows is fine

bhajneet commented 4 years ago

@AkalUstat Please accept the triage permission for desktop repo (you have to be logged in before clicking the link in your email). This will let you change the issue's status label from "to do" to "wip"

EDIT: Also please assign yourself haha

AkalUstat commented 4 years ago

@bhajneet the Mac workaround doesnt work very well. Since the fullscreen on each tab of the app is a mac default setting that each app has (and this issue doesnt appear on windows), im leaving it as it and opening up the pr

bhajneet commented 4 years ago

@Harjot1Singh consider dropping support for mac on this option? Only make it available to windows users?

Harjot1Singh commented 4 years ago

Not sure. I think the issue is more around the semantics of fullscreening on Mac generally, as opposed to this particular feature, so I would suggest merging it in, but think about/investigate how our app actually works in terms of fullscreening on Mac.

saihaj commented 4 years ago

Doing this also makes all other items fullscreen too. Like if you open settings (and this settings is enabled) it is in fullscreen mode. Is this expected?

Kapture 2020-06-16 at 11 27 54

AkalUstat commented 4 years ago

yes this is the mac default...windows does not expereince this

Harjot1Singh commented 4 years ago

Using simple fullscreen on mac produces the desired behaviour and does not fullscreen any sub-windows. @saihaj please confirm this works as expected now.

saihaj commented 4 years ago

Yep, that works. Do we not want it full screen by default?

Kapture 2020-06-28 at 16 36 52

Harjot1Singh commented 4 years ago

We do not want it to fullscreen by default