Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Implement electron-zone #537

Closed
robwormald opened this issue Dec 11, 2016 · 26 comments · Fixed by #983
Closed

Implement electron-zone #537

robwormald opened this issue Dec 11, 2016 · 26 comments · Fixed by #983

Comments

@robwormald
Copy link
Contributor

Electron uses both browser and some node APIs (EventEmitter, etc), and events fired by electron APIs (MenuItem, for example) cause angular to drop out the zone. Ref angular/angular#13363

We should implement an electron zone patch, that combines the browser APIs as well as a subset of the node APIs available to electron.

@JiaLiPassion
Copy link
Collaborator

I tried the issue in this repo, but can't reproduce. But I think current zone.js did not patch browser and node.js at the same time, just like #524 maybe the same issue in electron. Could you provide a reproduce step?

@robwormald
Copy link
Contributor Author

@JiaLiPassion navigate via the menu at the top of the screen:
screen shot 2016-12-11 at 10 25 07 pm

@JiaLiPassion
Copy link
Collaborator

JiaLiPassion commented Dec 11, 2016

I cleaned up my node_modules, I can reproduce now.

@sittingbool
Copy link

any update on this? i can use angular 2 only up to version 2.1 so far with electron

@JiaLiPassion
Copy link
Collaborator

For now, you have to do like this

 let pageMenu = new MenuItem({
            label: 'Page',
            submenu: [
                { label: 'Page 1', click: Zone.current.wrap(() => this.router.navigate(['/page1'])) },
                { label: 'Page 2', click: Zone.current.wrap(() => this.router.navigate(['/page2'])) },
            ],
        });

@sittingbool
Copy link

I don't only have a problem with the router. I have issues in general like with change detection and such.

@JiaLiPassion
Copy link
Collaborator

@sittingbool, could you give an example through some sample code or repository.

@JiaLiPassion
Copy link
Collaborator

@sittingbool , if your problem is related with electron menu or something like that, you could use the
method just like

 let pageMenu = new MenuItem({
            label: 'Page',
            submenu: [
                { label: 'Page 1', click: Zone.current.wrap(() => this.router.navigate(['/page1'])) },
                { label: 'Page 2', click: Zone.current.wrap(() => this.router.navigate(['/page2'])) },
            ],
        });

If your problem is related with your application(not electron), you may check the #533 to see whether it is the same problem, if it is , you can check PR #608 to see whether it resolve your problem or not.

@pamtbaau
Copy link

pamtbaau commented Aug 9, 2017

Any progress on this issue with electon menus and zone?

@sittingbool
Copy link

I really would appreciate that too
I cant really use electron with Angular 2+ so far.

@pamtbaau
Copy link

pamtbaau commented Aug 9, 2017

@sittingbool The suggested workarround of wrapping my code with zone.run(...) works fine for me, but I would rather do it without...

This is how my code looks like when invoking an Electron menu item:

private initMenu() {
   const template: Electron.MenuItemConstructorOptions[] = [
      {
          label: 'Set exchange rates',
          click: () => this.zone.run(() => this.showExchangeRateDialog = true),
      },
      {
          label: 'Set stock prices',
          click: () => this.zone.run(() => this.showStockPriceDialog = true),
      },
   ];

   this.menu = Menu.buildFromTemplate(template);
}

ChangeDetector is now picking up any changes of this.showExchangeRateDialog and this.showStockPriceDialog and the dialogs will show accordingly.

@sittingbool
Copy link

I don't have this Issue with menu alone. I have a general Issue with the Zone- Implementation in electron. The behavior has been overwritten from the default as far as I could see and Zone.js is reacting allergic to it.

@JiaLiPassion
Copy link
Collaborator

@sittingbool , could you post a reproduce repo?

@JiaLiPassion
Copy link
Collaborator

@pamtbaau , if it is just menu issue, I can make a patch to do it, but I don't know there are other cases or not.

@pamtbaau
Copy link

@JiaLiPassion, So far, I've encountered the problem when setting a property inside an Electron initiated callback. E.g. a (popup)menu callback or dialog callback. I've created a repo to reproduce the three scenarios.

@daganchen
Copy link

Having the same issues like @sittingbool,
all events are firing but component not rendering as expected.
I'm using Angular5, if I downgrade to version 2, issues will stop appearing?

@JiaLiPassion
Copy link
Collaborator

@daganchen , this issue has nothing to do with angular version, I am implementing an electron zone now, please wait for a while.

@ameagol
Copy link

ameagol commented Dec 12, 2017

Hi All, I've just found a solution that works for Angular 5 and all latest Codes for Node.js and Typescript:

Follow:

npm install ngx-electron --save

then, use the code as follow

import { ElectronService } from 'ngx-electron';
import { Component, OnInit } from '@angular/core';
import { Router } from '@angular/router'; 


@Component({
  selector: 'app-home',
  templateUrl: './home.component.html',
  styleUrls: ['./home.component.css']
})
export class HomeComponent implements OnInit {
 
  constructor(private _electronService: ElectronService,private router: Router) {  
    this.initMenu();
  }

  ngOnInit() {
 

  }
 
    private initMenu() {
      let menu = this._electronService.remote.Menu.getApplicationMenu();

      let pageMenu  = new this._electronService.remote.MenuItem(
        {
          label: 'Page',
              submenu: [
                  { label: 'Page 1', click: () => this.router.navigate(['/page1']) },
                  { label: 'Page 2', click: () => this.router.navigate(['/page']) },
              ],
        }
      );

      menu.insert(1, pageMenu);

      this._electronService.remote.Menu.setApplicationMenu(menu);
  
    }
}

Make Sure the pages exists on routing file, otherwise will throw many route errors.

More details check Wonderfull Lib from ThorstenHans here

@pamtbaau
Copy link

@ameagol I'm not using Angular's routing, but the above mentioned workaround works fine for the original issue. Maybe the workaround might also work in your use case, which might make the extra library a bit superfluous for an issue that will be properly fixed in due time.

I use menus in the following way:

...
import { remote } from 'electron';
const { Menu } = remote;

@Component({
    selector: 'main-component',
    templateUrl: 'main.component.html',
})

export class MainComponent implements OnDestroy {
    private menu: Electron.Menu;
    ...
    constructor() {
        this.initMenu();
    }

    public onMenu() {
        this.menu.popup();
    }

    private initMenu() {
        const template: Electron.MenuItemConstructorOptions[] = [
            {
                label: 'Set exchange rates',
                click: () => this.zone.run(() => this.showExchangeRateDialog = true),
            },
            {
                label: 'Set stock prices',
                click: () => this.zone.run(() => this.showStockPriceDialog = true),
            },
        ];

        this.menu = Menu.buildFromTemplate(template);
    }
}

@pamtbaau
Copy link

@JiaLiPassion Great news! Thanks for this zone fixup for Electron. Would love to test this out in my application. When will it be released and how should I use it?

@JiaLiPassion
Copy link
Collaborator

@pamtbaau, thank you, I have created a sample repo to describe how to use, https://github.com/JiaLiPassion/zone-electron, please use the new release to test your application.

@pamtbaau
Copy link

Maybe I have a wrong perception as to how this feature solves the electron zone issues.

The next piece of code is a declaration of a menuitem using this.zone.run() as workaround:

const template: MenuItemConstructorOptions[] = [
    {
        label: `Delete ${hero.name}`,
        click: () => this.zone.run(() => this.deleteHero(hero)),
    },
];

I hoped I could now simply remove the this.zone.run() wrapper:

const template: MenuItemConstructorOptions[] = [
    {
        label: `Delete ${hero.name}`,
        click: () => this.deleteHero(hero),
    },
];

But no luck... Without the this.zone.run(), changeDetection doesn't work.
Could you please give me some hints as to how a menuitem should be declared right now?

@JiaLiPassion
Copy link
Collaborator

@pamtbaau , I have tested the menuItem.click(), it works, so could you check here,
https://github.com/JiaLiPassion/zone-electron/blob/master/src/app/components/home/home.component.ts#L108

and make sure

  1. you updated to zone.js 0.8.19
  2. in your src/polyfill.ts, add import 'zone.js/dist/zone-patch-electron'.

@pamtbaau
Copy link

@JiaLiPassion I have tested your suggestions, but it's not working

  1. Downloaded your app: It works
    Noticed that you are using package git+https://github.com/JiaLiPassion/zone.js.git#electrondist.
    NB. When installing your package, npm returns: +zone.js 0.8.18
  2. npm install --save zone.js (yields 0.8.19)
    You app doesn't work
  3. npm install --save git+https://github.com/JiaLiPassion/zone.js.git#electrondist
    You app works...

Same with my own app. It only works when installing git+https://github.com/JiaLiPassion/zone.js.git#electrondist

@JiaLiPassion
Copy link
Collaborator

@pamtbaau , thank you for your detail information, it seems 0.8.19 not release the newest contents, I will check it, please use git+https://github.com/JiaLiPassion/zone.js.git#electrondist for test now.

@JiaLiPassion
Copy link
Collaborator

@pamtbaau , I have checked 0.8.19 and found I made a typo, I will fix it and please use my branch for test by now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants