Skip to content

respect width/height (no fullscreen) if set in electronOptions...#3187

Merged
rejas merged 1 commit intoMagicMirrorOrg:developfrom
khassel:electron-fullscreen
Sep 13, 2023
Merged

respect width/height (no fullscreen) if set in electronOptions...#3187
rejas merged 1 commit intoMagicMirrorOrg:developfrom
khassel:electron-fullscreen

Conversation

@khassel
Copy link
Collaborator

@khassel khassel commented Sep 11, 2023

... in config.js.

Solves #3174

With getting width/heigt from electron.screen.getPrimaryDisplay().workAreaSize introduced with #3161 the solution was to remove the setFullscreen line.

So per default the fullscreen resolution is used but when someone now uses electronOptions.width/electronOptions.height in config.js these parameters are used and so "no fullscreen" is possible.

@codecov-commenter
Copy link

Codecov Report

Merging #3187 (f2667a8) into develop (f2957f9) will increase coverage by 0.08%.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##           develop    #3187      +/-   ##
===========================================
+ Coverage    25.22%   25.31%   +0.08%     
===========================================
  Files           54       54              
  Lines        11852    11851       -1     
===========================================
+ Hits          2990     3000      +10     
+ Misses        8862     8851      -11     
Files Changed Coverage Δ
js/electron.js 0.00% <ø> (ø)

... and 2 files with indirect coverage changes

@khassel khassel force-pushed the electron-fullscreen branch from f2667a8 to 4cf3c7b Compare September 13, 2023 20:54
@rejas rejas merged commit fa7c7fc into MagicMirrorOrg:develop Sep 13, 2023
@khassel khassel deleted the electron-fullscreen branch September 13, 2023 21:05
@bugsounet
Copy link
Contributor

Really!?

@bugsounet
Copy link
Contributor

@khassel @rejas

I have this:

image

It will not set fullscreen and I have my taskbar

that why I suggest: mainWindow.setFullScreen(config.fullscreen);

@khassel
Copy link
Collaborator Author

khassel commented Sep 13, 2023

you are right, I tested this on a system without taskbar, will look again into this ...

@rejas
Copy link
Collaborator

rejas commented Sep 13, 2023

Shall we revert this?
And: Do you think its possbile to write a test for this usecase?

@khassel
Copy link
Collaborator Author

khassel commented Sep 13, 2023

Shall we revert this?

no, will provide another PR later

And: Do you think its possbile to write a test for this usecase?

no idea at the moment

@bugsounet
Copy link
Contributor

bugsounet commented Sep 13, 2023

(revert not sure, You have commit something else after)

@khassel will give us something better in the next commit. I trust ;)

rejas pushed a commit that referenced this pull request Sep 14, 2023
follow up for #3187

@bugsounet can you please confirm that this now works for you?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants