AlexTorresDev / custom-electron-titlebar

Custom electon title bar inpire on VS Code title bar
MIT License
868 stars 149 forks source link

Default Parameters For onMinimize, onMaximize, etc. #174

Closed yikuansun closed 2 years ago

yikuansun commented 2 years ago

Describe the bug I'm a little confused about why this is an issue, considering that older releases of custom-electron-titlebar did not require developers to write out the code to make the titlebar buttons work in the normal, standard way. However, in v4.0.1, I can't leave parameters like onMinimize and onMaximize blank without errors. I'm not too familiar with TypeScript, but the parameters should default to the following:

    onMinimize: function() { BrowserWindow.getFocusedWindow().minimize(); },
    onMaximize: function() { BrowserWindow.getFocusedWindow().isMaximized()?BrowserWindow.getFocusedWindow().unmaximize():BrowserWindow.getFocusedWindow().maximize(); },
    onClose: function() { BrowserWindow.getFocusedWindow().close() },
    isMaximized: function() { return BrowserWindow.getFocusedWindow().isMaximized() },
    onMenuItemClick: function() { console.log("nothing"); }

To Reproduce

var titlebar = new customTitlebar.Titlebar({
    backgroundColor: customTitlebar.Color.fromHex("#474747"),
    menu: null
}); // used to work - no longer does

Expected behavior custom-electron-titlebar will know that since onMinimize, onMaximize, etc. were left blank, it should make the default buttons.

Desktop (please complete the following information):

AlexTorresDev commented 2 years ago

Hi @yikuansun. Due to the change made by electron to stop using remote, the package has been migrated to IPC, so it is mandatory to define these functions. Since, you no longer have the possibility of accessing the windows remotely as before.

Because. If I use BrowserWindow.getFocusedWindow() in my package this returns an undefined and the action will never happen.

Araxeus commented 2 years ago

right now custom-electron-titlebar only works in the renderer process right? that's what causing the problems

maybe you could move the whole titlebar constructor to the Main process and do all those ipc events internally?

EDIT:

or even easier: create a an export in index.ts like setupMain

import { setupMain } from './main';

export = { Titlebar, Color, setupMain };

and inside that setupMain.ts file check that we are indeed in main process

if (process.type !== 'browser') return;

and then implement all that is here https://github.com/AlexTorresSk/custom-electron-titlebar/blob/4d7c6aef2cdf4bd4e588b6c54acb2cbe25829a94/example/titlebar-events.js#L4-L39

Then users could just require it once in the main process

require('custom-electron-titlebar').setupMain();
yikuansun commented 2 years ago

Thanks @AlexTorresSk for the clarification, and thanks @Araxeus for the proposed solution. I didn't realize that'd it'd be possible, but using the constructor from the main process is definitely a lot cleaner and more robust.

Araxeus commented 2 years ago

Well dunno why this wasn't closed automatically... the linked pull request was merged so pretty weird

anyways I fixed this issue in #175

(can now just require a main script in main process once on load + another one to attach to BrowserWindow - see example in https://github.com/Araxeus/custom-electron-titlebar/tree/v4.1.0/example)