Desktop: fix window dimensions and position when the application start #2514

daukadolt · 2020-02-16T19:06:10Z

Addresses #2476

BrowserWindow set up to have a minimum width and height of 100 px. This is addressed by setting minWidth, minHeight properties of BrowserWindow.
Centered position on a primary display is enforced if BrowserWindow rectangle doesn't match any display.

Electron app doesn't allow scaling any lower than minHeight and minWidth, which I've checked. However, I could not enforce the application to have a position outside my only display both through setting x, y manually here or through BrowserWindow.setPosition(). Hence, I could not check my solution on this one.

I initially wanted to discuss my approach (like setting arbitrary 100px for minHeight/minWidth) in the issue, but (sorry if I'm wrong) I thought that since pull request would either way be reviewed discussion could be set here.

Looking forward for comments.

tessus

I think we could try to corrupt the window-state-prod.json on purpose. On macOS, this file is in ~/Library/Application Support/Joplin. Not sure about Linux and Windows.

tessus · 2020-02-17T23:07:57Z

This is a great start, but to bring it to the next level, I'd suggest the following: make the default size and position dependent of the screen size. e.g. that it takes 80%-85% of the width and height and that it is centered. This is just an idea and not necessary.
This is just a workaround for people who run into the weird problem of a 1x1 sized window or a window off screen (due to multiple monitors).

gasolin · 2020-02-18T13:12:53Z

set minWidth/minHeight also solves the user occasionally scale the window to a minimum line issue.

https://discourse.joplinapp.org/t/installing-joplin-1-0-174-on-macos-catalina-results-in-no-visible-window/4425/10

@daukadolt I suggest to set the minWidth: 480, minHeight: 320 to keep the Joplin window barely workable

daukadolt · 2020-02-20T19:10:34Z

@tessus I've made default width and height dependent on the primary screen, since that is the screen that the app is spawned on for the first time. However, Travis build has failed and I wonder why. After a quick run through the logs and repo it seems that the first line intended for eslint is the culprit, but how come did the master repo build successfully? Or my commit somehow messes with it. And why is the build performed on laurent22/master instead of my master branch?

Additionally, I tried playing around with window-state.json file, but it's BrowserWindow constructor that is safeguarding against spawning outside any display, I believe.
electron-window-state package too has some safeguards against spawning outside any window, but I've even modified it's code in node_modules by hardcoding return of 4000 here for x and y, which is why I believe that BrowserWindow blocks spawning outside window.

tessus · 2020-02-20T19:26:17Z

Right now all builds fail in Travis. We have to fix that. So don't worry about it right now.

I'll have a look at your new commit either tonight or tomorrow.

daukadolt · 2020-02-21T19:41:03Z

@gasolin I like your suggestion, I will commit updated minimum dimensions after all other issues will be resolved

tessus · 2020-02-22T20:50:14Z

Please rebase, so that the conflicts are resolved. This will also solve the Travis builds. The build process has changed, so everything should be fine now. Please have a look at the updated BUILD.md file.

Hardcoded minimum width and height of 100 px. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

Set Electron window at the center of primaryDisplay if BrowserWindow is not on any displays. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

Set Electron window defaultWidth, defaultHeight to 80% of primary display workArea width, height, respectively. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

daukadolt · 2020-02-23T13:05:24Z

Please rebase, so that the conflicts are resolved. This will also solve the Travis builds. The build process has changed, so everything should be fine now. Please have a look at the updated BUILD.md file.

Got it. I've checked the updated test steps. What do you think about @gasolin 's suggestion on minWidth/minHeight? I can change them if everything else is fine.

Hotfix of a typo that calculates Electron app's default height at first run using primary screen's width, not height. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

daukadolt · 2020-02-23T18:01:45Z

Found a typo in my code, sorry for the sloppiness. However, here again, BrowserWindow safeguards against too big of a height. Doing 2*currentWorkArea.heightstill renders app within my mac's desktop, not going any lower than the dock and not higher than the menu bar. Tried resizing dock as well.

daukadolt · 2020-02-24T14:43:19Z

@PackElend could you please tag this PR as gsoc-2020 as well?

tessus

LGTM, but I'd change the defaultWidth and defaultHeight to something more sane.
Or depending on the actual screen size.

daukadolt · 2020-02-25T08:24:51Z

LGTM, but I'd change the defaultWidth and defaultHeight to something more sane.
Or depending on the actual screen size.

@tessus you mean using screen size instead of workArea?

tessus · 2020-02-26T02:59:17Z

I meant that if your screen is e.g. 1024x768, use ca 80-85% of that for the window size. Maybe 800x600 and position it (1024-800)/2 , (800-600)/2 so that it is in the middle of the screen.

This should of course only be done, if there's a problem with the size or the position in the first place. If the data in window-state-prod.json is correct, you don't have to do anything.

laurent22 · 2020-02-26T23:33:20Z

ElectronClient/ElectronAppWrapper.js

@@ -43,9 +43,12 @@ class ElectronAppWrapper {

 		const windowStateKeeper = require('electron-window-state');

+		const currentWorkArea = screen.getPrimaryDisplay().workArea;
+		const [defaultWidth, defaultHeight] = [0.8*currentWorkArea.width, 0.8*currentWorkArea.height];


Please round the values. Also I don't see a need to create these separate values, so if you could set the properties on stateOptions directly that would be best.

@laurent22 can you take a look at the changes?

laurent22 · 2020-02-26T23:36:56Z

Just left one comments. @tessus, is there something more that needs to be changed? I see he already uses 80% of the space or did I miss something?

tessus · 2020-02-26T23:53:12Z

No, @laurent22, all is good. The OP has already changed the code according to my suggestion.

laurent22 · 2020-02-26T23:55:50Z

Perfect, then there's just this one minor change to make and we can merge.

Setting defaultWidth/defaultHeight directly without extra variables. Rounding defaultWidth/defaultHeight. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

scotrod · 2022-01-29T20:24:35Z

Bump. This is still happening with Joplin 2.6.10 (prod, win32)

Client ID: cf27eb0272a5451ca45a96eaa6351910
Sync Version: 3
Profile Version: 41
Keychain Supported: Yes

Revision: 98fba37

This issue appeared only on my multi-monitor setup - after I deleted the folder in C:\Users\USER\AppData\Roaming\Joplin\ it worked like a charm.

tormung83 · 2023-04-26T20:22:52Z

@scotrod and all, this is still happening on Multi monitor and laptop's main display, docked and undocked.
Deleting the C:\Users\USER\AppData\Roaming\Joplin\ did not make any difference.

OS: Windows 11 22H2 (OS Build 22621.1555)
Joplin version: 2.9.17

daukadolt requested review from laurent22 and tessus February 16, 2020 19:06

tessus linked an issue Feb 17, 2020 that may be closed by this pull request

Check and fix window dimensions and position when the application start #2476

Closed

tessus added the desktop All desktop platforms label Feb 17, 2020

tessus reviewed Feb 17, 2020

View reviewed changes

gasolin mentioned this pull request Feb 18, 2020

Desktop: set min width/height window size #2521

Closed

daukadolt added 3 commits February 23, 2020 18:21

ElectronClient window minimum size set

3110f31

Hardcoded minimum width and height of 100 px. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

Set Electron window position if outside displays

29df6fc

Set Electron window at the center of primaryDisplay if BrowserWindow is not on any displays. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

Set Electron window default size dependent on screen

9081867

Set Electron window defaultWidth, defaultHeight to 80% of primary display workArea width, height, respectively. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

Hotfixing Electron defaultHeight depending on width, not height

de7a510

Hotfix of a typo that calculates Electron app's default height at first run using primary screen's width, not height. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

daukadolt requested a review from tessus February 24, 2020 09:08

PackElend added the gsoc-2020 label Feb 24, 2020

tessus reviewed Feb 24, 2020

View reviewed changes

laurent22 requested changes Feb 26, 2020

View reviewed changes

Refactoring Electron app defaultWidth/defaultHeight setup

b9e3287

Setting defaultWidth/defaultHeight directly without extra variables. Rounding defaultWidth/defaultHeight. Signed-off-by: daukadolt <daulet.amirkhanov.official@gmail.com>

laurent22 merged commit ec64bf2 into laurent22:master Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Desktop: fix window dimensions and position when the application start #2514

Desktop: fix window dimensions and position when the application start #2514

daukadolt commented Feb 16, 2020

tessus left a comment

tessus commented Feb 17, 2020

gasolin commented Feb 18, 2020

daukadolt commented Feb 20, 2020 •

edited

Loading

tessus commented Feb 20, 2020

daukadolt commented Feb 21, 2020

tessus commented Feb 22, 2020 •

edited

Loading

daukadolt commented Feb 23, 2020

daukadolt commented Feb 23, 2020

daukadolt commented Feb 24, 2020

tessus left a comment •

edited

Loading

daukadolt commented Feb 25, 2020

tessus commented Feb 26, 2020

laurent22 Feb 26, 2020

daukadolt Feb 27, 2020

laurent22 commented Feb 26, 2020

tessus commented Feb 26, 2020

laurent22 commented Feb 26, 2020

scotrod commented Jan 29, 2022

tormung83 commented Apr 26, 2023

Desktop: fix window dimensions and position when the application start #2514

Desktop: fix window dimensions and position when the application start #2514

Conversation

daukadolt commented Feb 16, 2020

tessus left a comment

Choose a reason for hiding this comment

tessus commented Feb 17, 2020

gasolin commented Feb 18, 2020

daukadolt commented Feb 20, 2020 • edited Loading

tessus commented Feb 20, 2020

daukadolt commented Feb 21, 2020

tessus commented Feb 22, 2020 • edited Loading

daukadolt commented Feb 23, 2020

daukadolt commented Feb 23, 2020

daukadolt commented Feb 24, 2020

tessus left a comment • edited Loading

Choose a reason for hiding this comment

daukadolt commented Feb 25, 2020

tessus commented Feb 26, 2020

laurent22 Feb 26, 2020

Choose a reason for hiding this comment

daukadolt Feb 27, 2020

Choose a reason for hiding this comment

laurent22 commented Feb 26, 2020

tessus commented Feb 26, 2020

laurent22 commented Feb 26, 2020

scotrod commented Jan 29, 2022

tormung83 commented Apr 26, 2023

daukadolt commented Feb 20, 2020 •

edited

Loading

tessus commented Feb 22, 2020 •

edited

Loading

tessus left a comment •

edited

Loading