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

Multiple instances conflict, but only a little #199

Open
lfeigen opened this issue Aug 28, 2023 · 4 comments
Open

Multiple instances conflict, but only a little #199

lfeigen opened this issue Aug 28, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@lfeigen
Copy link

lfeigen commented Aug 28, 2023

Describe the bug
In our project, we have multiple preferences-like pages. In particular, we have a "Settings" page and a "Preferences" page. Obviously, settings refers to things like "proxy address" which is hardly a "preference" if you can't possibly work without it. "Preferences", on the other hand, is for things like use of confirmation dialogs.

One can't have multiple instances of electron-preferences since the ipcMain.on() registrations conflict. We have worked around this in our code, by doing ipcMain.removeAllListeners() liberally. But a better approach would be to take down the listeners upon "close" or to support a "destroy" API.

I lean towards the "destroy" approach, as it allows some control for the user of the API. If you have only one, there may be no reason to keep creating and destroying things.

Also, the destroy API can do things like getting rid of the prefsWindow object and other wasteful things.

To Reproduce

  1. Call new ElectronPreferences() twice
  2. Cause a "show" on the first instance.
  3. Close it
  4. Cause a show on the second instance.
  5. You will see the contents of the first instance on the screen, not the second.

Expected behavior
It would be expected that each instance would operate independently.

@lfeigen lfeigen added the bug Something isn't working label Aug 28, 2023
@lacymorrow
Copy link
Collaborator

Yep, this is an issue for sure, would require a bit of a refactor

@pvrobays
Copy link
Collaborator

pvrobays commented Sep 6, 2023

By design the electron-preferences is created to be used as a single instance.
I'm quite sure we should be able to make it capable of running as multi-instance but that brings along quite some complexity and needs thorough testing.

Can I ask why you have the architecture of both settings and preferences within two different windows, and not just one window which has both the preferences and the settings in it, albeit in different tabs (groups)?

If possible, I would go for the single window design which should fix your issue, and probably makes your application more simple and elegant?

@lfeigen
Copy link
Author

lfeigen commented Sep 7, 2023

We went back and forth on whether they are two things or one. I can give you examples of applications that make each choice.

For us, there seemed to be a clear difference. Setting a proxy, is not a "preference" in any sense. If you don't set the proxy server when you need it, you can't connect. On the other hand, if you don't like confirmation dialogs, that is just a matter of taste.

How does this logical distinction play out? Well, here's an example. We are planning on having a way of sharing settings, so that if a team all needs a certain setting (e.g. the whole team needs to use a certain proxy server), they can share it. But, clearly, sharing preferences is silly.

The distinction also plays out in documentation, of course.

Could we use a single "settings/preferences" page with different tabs, where some tabs are "settings-like" and others are "preferences-like"? Of course, we could. But I'm comfortable that we made the right choice in doing it this way.

We have kludged around the issue for now by writing our own destroy().

As I mentioned in another issue, I hope to be able to provide a PR, if the fates allow.

@lacymorrow
Copy link
Collaborator

I actually had a use-case for two separate windows as well a couple years ago and ended up creating a custom dialog window.

This would definitely be beneficial.

@pvrobays pvrobays added enhancement New feature or request and removed bug Something isn't working labels Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants