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

The opened windows on Windows does not have the right height #44

Closed
jbdamiano opened this issue Sep 20, 2019 · 14 comments
Closed

The opened windows on Windows does not have the right height #44

jbdamiano opened this issue Sep 20, 2019 · 14 comments

Comments

@jbdamiano
Copy link

jbdamiano commented Sep 20, 2019

On Windows the displayed prompt is too small. On OS X the displayed prompt is correct.

image

@p-sam
Copy link
Owner

p-sam commented Sep 20, 2019

Could you show the code used to invoke the prompt window?

@jbdamiano
Copy link
Author

jbdamiano commented Sep 20, 2019

const ePrompt = require('electron-prompt');

ePrompt({
     title: 'Enter the Terminal Profile Data',
     label: 'data:',
     value: ''
})
 .then((r) => {
     if(r !== null) {
        //Send a message to decode the parameter
        //win.openDevTools();
        win.webContents.send('decode', {
            data: r`
         });
     }
})
.catch(console.error);

@Fraasi
Copy link
Contributor

Fraasi commented Oct 1, 2019

I remember this happening to me too. If you don't set the height, it defaults to 130, which is not enough even for this kind of simple input window on windows. My guess is that on OS X the font/input box/buttons/padding or whatnot is different size than on windows and everything fits snuggly.

Probably only way around this is to check which platform you are on and set height accordingly.
Maybe it wouldn't be too difficult to change the fixed default height to fit to content instead?

@p-sam
Copy link
Owner

p-sam commented Oct 1, 2019

I'd probably prefer a solution that would just set a window client size instead of trying to find the correct window size (including borders) on each platform.

@Fraasi
Copy link
Contributor

Fraasi commented Oct 1, 2019

Yeah, that was my tired attempt to explain on the last line 😩 To set the window height automatically according to the content the default behaviour. I'll try to find time to play around with this in the next few days and see if it's easy to implement.

@Fraasi
Copy link
Contributor

Fraasi commented Oct 2, 2019

Ok, it seems there's something weird going on with my win10 machine.
Running the latest electron quick start and electron-prompt 1.3.1.
If you leave the height out, prompt should default to 130, but it's around 80 pixels...

pic2

If you put the height to 330, it's only around 280...

pic1

So where's the missing 50 pixels:thinking:?

As a side note, I wrote this little function about a year ago for one of my apps to resize the electron window height to fit all the content in the window without scroll bars. Might be useful when thinking a solution for this issue.

@Fraasi
Copy link
Contributor

Fraasi commented Oct 3, 2019

Found the missing pixels, culprit is the BrowserWindow option useContentSize , if set to false, I get the default 130 pixels for the height, but as you can see, it's not enough...

pic3

Either there's some weird conflict with the settings or this is a bug in electron?
Yea, related electron issue

@Fraasi
Copy link
Contributor

Fraasi commented Oct 3, 2019

Ok, after reading the electron issue and testing, simply setting the 'resizable' to true solves this issue

pic4

So, I would propose that we change the resizable-option default to true, @p-sam ?

@p-sam
Copy link
Owner

p-sam commented Oct 4, 2019

Yeah, but as the option states, that would also make the window resizable, which is not desirable. You seem to want to set the width and height dynamically based on the content, and as I stated in my previous message, the ideal solution would be to set something commonly called as the "client size", which is the inner size of the window content (without chrome/border). I may look into this myself later, however I have other stuff on my TODO right now, sorry

@Fraasi
Copy link
Contributor

Fraasi commented Oct 6, 2019

No worries, I was just curious about what caused this behaviour. Anyway, it's should be pretty easy for others to 'fix' this now if they bumb into this problem.

@p-sam
Copy link
Owner

p-sam commented Oct 7, 2019

Please check if everything is resolved on develop. The actual issue was that the minimum height changed for all platforms (and was wrong), however not all platforms would exhibit the same behavior with different values of resizable/useContentSize. I'll probably release a new version I have multiple confirmations that it works now. (Tested myself on Windows and OSX right now)

@Fraasi
Copy link
Contributor

Fraasi commented Oct 11, 2019

Ok, glad to hear you found the actual problem. Comparing the master and developement branches, I see there is few new options and a lot of housecleaning done. Looks good to me for the next release 🚀

@p-sam
Copy link
Owner

p-sam commented Oct 11, 2019

alright, let's go then

@p-sam
Copy link
Owner

p-sam commented Oct 11, 2019

fixed in v1.4.0

@p-sam p-sam closed this as completed Oct 11, 2019
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

No branches or pull requests

3 participants