feat: Basic context menu

[mitchcapper]

started off super simple and honestly half this code is in the wrong place but a small concise example of a very extendable context menu. Works in browser, works in electron. Context menu code is MIT licensed so I believe should be good there.

Closes Related to https://github.com/httptoolkit/httptoolkit/issues/212

The context menu lib https://github.com/fkhadra/react-contexify supports icons, theming, etc so seems like it should be a good fit. POC implements body copy and pinning for the one standard list view item.

The webkit hack is due to: https://github.com/vuejs/pinia/issues/675 there may be a better way.

Personally, aside from the items already mentioned in other tickets for context menu items I think things like right clicking and pinning parts of requests/responses (ie a specific header, content length, etc) and then having the request/response cards when collapsed only show the pinned items. Would make it much quicker to potentially look at just the data you want from request to request.

Obviously the myriad of automatic rule adding options (right click on a header add a rule requiring that header, etc).

Anyway I figured get a basic version in first that is acceptable and go from there.

[socket-security[bot]]

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
react-contexify	6.0.0	None	+1	156 kB	sniphpet

[pimterry]

Hmmmm. I have mixed feelings. This is definitely a good way to quickly get this working! That said, I'd really like to use native UI where possible, and it is possible to use native platform context menus via Electron...

But supporting both is probably helpful - that way we can use Electron's context menu wherever possible, or fall back to a local HTML context menu the rest of the time.

The Electron API would be async & imperative (we have to pass & listen for messages) and we can't define it in terms of JSX (though internally the browser version could certainly use JSX to render). At the top level it would have to look something like this:

// An event callback, called when a row is right clicked:
function onRowContextMenu(exchange) {
  // Calls some kind of global API, or a provider injected via React context etc:
  openContextMenu(
    { exchange }, // Some data we can hold & pass back to item click listeners later
    [ // The menu to show:
        { type: "item", "id": "toggle-pinned", text: "Toggle Pinned" }
        { type: "separator" }
        { type: "submenu", label: "Copy", items: [
          { type: "item", "id": "copy-decoded-body", text: "Decoded Body" }
        ] }
    ],
    onContextMenuClick // The callback
  );
}

function onContextMenuClick(menuData, itemId) {
  // Do something for the clicked item
}

On the Electron side, it would show that menu using the native UI, at the current mouse position, and send a message back (calling the callback) if one was clicked. The UI just needs to call the latest registered callback with the id & data if that happens.

On the browser side, if Electron isn't available, we'd use react-contextify as here, passing that data to a React component that renders at the current position. If an item gets clicked, it does the same.

Maybe we could even replace the ids with just registering a callback on each item... Dunno, I can see arguments either way.

What do you think?

I'm open to this, but it would need to fit into that framework - as an API independent from specific UI components here (like the view list) where the definition is instead passed through as data, that we can translate into the two formats (HTML vs Electron API calls). That way we can potentially start with just react-contextify, but aim towards using Electron APIs instead where possible in future without a major refactor.

[mitchcapper]

I don't mind refactoring to a base interface and having the two implementations but what benefits are there to a native context menu? I would assume some minor airspace improvements (although it looks like contextify works around most of the ones one would normally run into). Aside from maintaining two code paths you have the downside that you are not going to be able to customize the context menu to nearly the same extent (at least not without a lot of extra code).

Happy to go down that route, just confirming there is some benefit to having a native option as well.

[pimterry]

Happy to go down that route, just confirming there is some benefit to having a native option as well.

Yes, native UI is definitely preferred where it's easy to do (like here). There's quite a few users who strongly prefer native components, and in general they do fit in better with user expectations both visually and in UX (e.g. shortcuts, accessibility options) across all different operating systems. And of course it's simpler at runtime - the operating system manages all the UI and details for us, we just need to use the API.

In fact, now that I think about it, you'll run into issues here because we already have an automatic context menu that's applied just by the desktop shell, to provide standard OS options when you're using Electron, e.g. it shows 'Select all', 'Copy', 'Paste', and context-specific options like 'Save image as' if you right click any image. This definitely needs to integrate with that, as currently this will probably show both context menus on top of on another in some cases.

[mitchcapper]

Don't worry I can extend the default.

[mitchcapper]

ALR well while I interfaced this out and implemented the HTML version I think it was all for nothing.

This library not only allows for a pretty straightforward native context menu for electron but it easily can do the extending to preserve the default functionality you speak of https://github.com/sindresorhus/electron-context-menu . The problem is customization based on the item here is the master issue: https://github.com/electron/electron/issues/35632

its possible with a side channel one could do it semi efficiently with some swapping. I would say if much larger projects haven't found a way to do it efficiently it may be worth considering. The context menu here does not need to always be exposed either, that means one could still allow the native context menu (which could even be extended with the project linked) for things like text boxes, or images.

[pimterry]

This library not only allows for a pretty straightforward native context menu for electron but it easily can do the extending to preserve the default functionality you speak of

Yes - we already use this here (that repo has all the Electron parts).

I don't think it's a hard problem to integrate the two - the first comment in the issue you linked is indeed exactly what we want to do, and will work just fine: we would listen for context-menu events in the renderer (i.e. in this repo, in the web page), then manually trigger the Electron context events from there with the given menu items when the page has an opinion on what those should be (whenever they're not just the defaults), using preventDefault to effectively take responsibility in the UI for opening for context menu for that click. That's when running in Electron - in the browser we'd do the same but just render the menu via HTML.

I think you don't need to worry about the Electron part here, I'll sort it out separately. If you can refactor this towards an API like the one above (i.e. an API that defines the menu items as a data structure) then it is definitely possible to use that to handle the context menu via Electron later.

[mitchcapper]

OK here is a potential base interface. This is still very much just a draft as I have some example code in it, and am not familiar enough with react to know the best way to do the model/view separation. In theory I think the native one will require a proxy implementation of ContextMenu forwarding things to the external renderer but you probably know more on that one.

Interface is pretty basic to implement just needs:

• SetContextMenuItems - Pass an array of context menu items to create the menu for, while items can be set enabled/disabled visible/hidden at any point the general menu is defined once early on (for that specific menu type).
• AppendContextMenuItem - appends a single context menu item to the end of the current list
• GetContextMenuHandler - This returns a function for showing the context menu, it takes any opaque data object that you want passed back to you on click. Generally this is called for every instance of an item with your context menu to bind that item to it.
• GetMenuComponent() - This returns a react component that gets added to the page once per specific context menu. Likely this needs to be adjusted (for the native menu I am guessing you are going to need to bind an attribute value to the item the menu gets appended onto). Per the note below the way I would expect to do it didn't work.

There are 3 menu item types, all can be hidden, all but the separator can be disabled and have display labels:

• SeparatorMenuItem - just a separator bar
• SubMenuItem - A menu item that has child items
• ContextMenuItem - Standard menu item, has an onClick handler that will get the event and the bound data from GetContextMenuHandler

A few items I ran into that there may be a better way on:

• I originally wrote an AttachMenuToItem that would take a component and try to set onContextMenu to it but this pattern doesn't seem to be supported (other than by the discouraged clone option). Instead the item its being attached to specifically attaches directly. For the native context menu I am guessing this will not be needed.

• For the react component injection for the menu itself I use the .menu prop on the interface and inject it. Not sure if the native one needs to inject anything and while id assume returning empty is doable it may be better to check the result before just inserting.

• The interface module has SetNewContextMenuGenerator the idea being you can call this to override what contextmenu is used. Originally I tried to call this from the index.ts or app.ts file but it was not early enough (the deeper view-event-list.tsx got called before). Temporarily I moved to calling that from the HTMLContextMenu type, but the primary code in there doesn't get called unless something references it so I have a super stupid "noop()" call in view-event-list. Declaring the context menu globally as done let aMenu = GetNewContextMenu(); in that file is probably not the right time and might resolve this.

• The code used to get a context menu item by ID is not the most performant if there are very deep multi-level menu's, i doubt this would ever be a real issue but likely could pass additional data about the level of an item to avoid the depth first search done right now.

• I like strongly typed things but once I strongly typed the contextmenu data I had to convert the contextmenu to a generic. Finding good generic syntax was tricky. Originally ContextMenu was an interface, but I can't do extension methods on interfaces, (or default functions) but didn't want to duplicate broilerplate code. Instead I settled for making it an abstract class and then adding the NewSubMenuItem, etc helpers. This is to avoid you having to do the following when calling the context menu

  let aMenu = GetNewContextMenu<EventClickedData>();
  aMenu.AppendContextMenuItem(new SubMenuItem<EventClickedData>({title:"Hi Bob"}));

  It is having to repeat the EventClickedData I wanted to avoid but this is the only way I could get it to infer properly.

Again, the code certainly needs different organization, I could guess better spots but still feel they would be wrong until I get a better idea of the code space and pattern. I have a disable of sample code (for real time control of context menu items) first time you right click pin is enabled, second time disabled, etc. Clearly not something useful here but wanted to document the pattern I was expecting.

The copyToClipboard utility function in view-event-list.tsx doesn't go there vs some utility file, but not sure if it is desired or not. Mainly done to work around some of the default chrome restrictions on copying depending on host.

[mitchcapper]

OK I refactored this to use a strongly typed parameter system and updated my notes above. I like it more, implementers now have an easier time with the data passed).

I do have code as well ready for pin favorite / exclude certain headers but it depends on the context menu so I will wait to any version of this gets added then refactor and do it as its own PR. Here is what is working (based off this branch):

https://github.com/mitchcapper/httptoolkit-ui/compare/feature_context_right_click_menu...header_include_exclude_sample

[pimterry]

I started digging into this - it's a good start and there was a lot of helpful stuff here, but it didn't quite fit into the wider architecture so it needed a bit of refactoring. I got a bit carried away patching that, and then implementing & integrating a desktop native context menu to test that integration route, and then refactoring various other existing features and integrating them too, to enable all the other context menu buttons, but it's now all totally complete and working!

I'll merge this now. That will immediately make this available everywhere using the react-contexify implementation, and then for any future users who install the latest desktop version (coming shortly) it will automatically use the native context menu APIs instead.

Ironically, the one thing it doesn't do is to solve the original https://github.com/httptoolkit/httptoolkit/issues/212 issue - on closer reading, that's actually talking about right clicking on the body editor within the request/response, not in the list. That's an issue related to Monoco's own built-in context menu system, not to right clicking on individual rows. I think this will help solve it later, but it's a separately problem for now.

This does handle the other cases though, as listed in https://github.com/httptoolkit/httptoolkit/issues/162 and https://github.com/httptoolkit/httptoolkit/issues/69 - it includes all the menu options suggested there.

Thanks for contributing! I've been looking at doing this for ages and it's really great to finally get this added :+1: