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

chore(react-electron-menu): implement React based rendering for the various electron menus in the app 🦨 #5818

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

gribnoysup
Copy link
Contributor

@gribnoysup gribnoysup commented May 17, 2024

For this Skunkwors I hacked together something we discussed pretty long time ago and I wanted to try out ever since: this patch introduces a new package in the monorepo that should allow us to render Electron menu (either application menu, dock menu, or context one) using React components right in the application code.

The upsides of this approach is that we can use a more declarative way of defining the menus and bring them closer to the actual application features and data without the need of manually syncing required data back and forth between main process and renderers. Using current "Collection" menu items as an example, not only we don't need any extra ipc events to make main process aware of what is the current active tab in the app, whether or not it's a collection, and if this collection is read only, we have access to way more useful collection metadata that we can use to build the menu.

The library works somewhat similarly to react-helmet in a sense that if you are rendering submenus, they will be merged, allowing childred to declaratively add more items to already rendered items. This is for example how the "Connect" submenu works in this PR.

A new feature that we didn't have easy way of implementing before is that we can now render context menus dynamically right in React components that have access to all the context of the application around them. This makes implementing things like copying for the document or showing navigation tree item actions on right click very stragithforward.

There are some existing libraries out there that are trying to do the same thing, but all of them don't look maintained, they use "remote" as a way to communicate with main, which is not recomended anymore (deprecated and removed when used from electron directly), and they don't allow to define the menu on different levels of application tree which is a feature I think we can benefit from the most.

Some examples of things I described above:

Connection now shows the connection name and is dynamically rendered only when you are actually connected (supports multiple connections)

Kapture 2024-05-17 at 15 59 36

Collection menu uses the collection name in the menu group

Kapture 2024-05-17 at 15 29 35

Context menu for copying the document

image

Context menu for navigation tree items

image

I didn't have much time to work on this, so some polish would be required if we want to merge this into main:

  • Move the rest of the menus to the React render. I only moved a few for the examples
  • Add tests, clean up the code of the library a bit, I kinda rushed it, so there are probably ways to make it easier to read
  • Current implementation only allows one top level menu that will handle merged submenus (just because this seems to be our main case and was easier to implement), if we want for example the same thing for both dock and app menu simultaneously, we'd need to refactor this logic a bit

<ElectronMenuItem
label="&Import Data"
onClick={() => {
globalAppRegistry.emit('open-active-namespace-import');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still uses these special case active-namespace events because it was the fastest way to make it work, but as we now render these contextually, we can completely drop those types of events from the code as all the context is available here

@@ -18,6 +18,8 @@ import chalk from 'chalk';
import { installEarlyLoggingListener } from './logging';
import { installEarlyOpenUrlListener } from './window-manager';

Menu.setApplicationMenu(null);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really related to anything, but electron docs mention that this can speed up application start

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

Successfully merging this pull request may close these issues.

None yet

1 participant