Closed MathieuDebit closed 5 years ago
Hey there 😃 First, excellent works and welcome! Below I added some remarks on the global PR and specific comments attached as conversations.
For TS enums, we prefer a camelcase syntax as well for both the key and the values since it offers better visibility. e.g. enum WindowType { Normal, DevTools }
And warning about enum values, under the hood, you have written this block:
const WINDOW_TYPE = {
NORMAL = 0,
POPUP = 1,
DEVTOOL = 2,
};
which is the incorrect values since we want a representative string, so do something like this:
enum WindowType {
Normal = 'normal',
...
}
Instead of Electron native IPC calls, prefer use electron-simple-rpc
like this:
export type Callback = (payload: Window) => void;
export const CxApiHandler = 'cx-handler';
...
export enum CxApiChannels {
Windows = 'cx-api-windows',
...
}
export enum CxWindowsApi {
GetCurrent = 'get-current',
...
}
class ChromeWindowsAPIClient {
constructor(extensionId: string) {
this.scope = `${CxApiHandler}-${extensionId}`;
}
...
getCurrent(getInfo: GetInfo, callback: Callback) {
rpc(this.scope, CxWindowsApi.GetCurrent)(getInfo).then(callback);
}
...
}
class ChromeWindowsAPIHandler {
constructor(extensionId: string) {
const library = {};
[CxWindowsApi.GetCurrent, ...].forEach(channel => {
const handler = this[`handle${capitalize(channel)}`];
lib[channel] = (...args: any[]) => {
return handler.apply(this, args);
};
});
this.scope = `${CxApiChannels.Windows}-${extensionId}`;
this.RpcIpcManager = new RpcIpcManager(librabry, scope);
}
handleGetCurrent(getInfo: GetInfo) {
const wc = webContents.getFocusedWebContents();
return {
id: wc.id,
focused: wc.isFocused(),
incognito: false,
alwaysOnTop: false,
};
}
}
For the event management, please use a pattern like this one. Events from one extension should be broadcasted across all extensions listeners.
For RPC, don't forget to release the channel
The last thing, about the domain logic, at the API client level, maintain a map of created windows by the extension to allow updates and deletion of the created windows only.
Close for #34
https://www.notion.so/stationhq/chrome-windows-aec23de5db4748f08b1cafadb990528c https://developer.chrome.com/extensions/windows https://electronjs.org/docs/api/browser-window
TODO