snapcrunch / electron-preferences

A simple, consistent interface for managing user preferences within an Electron application.
MIT License
121 stars 30 forks source link

Multiple instances conflict, but only a little #199

Open lfeigen opened 1 year ago

lfeigen commented 1 year ago

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.

lacymorrow commented 1 year ago

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

pvrobays commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.