Allow Custom Tracks/Data Adapters from Plugins to Add Config Options to Track Menu Items

[jjrozewicki]

I apologize for the length of this. I have been working on this issue for many weeks, and I have finally been able to describe clearly the problem I'm having. There is a bug in the sense that it seems like JBrowse2 works differently than described in the documentation, but I'm also requesting a more substantial fix than just fixing that specific problem as I do not think it will fix the key issue.

Background

I am currently maintaining a simple plugin that calculates custom statistics from the reference sequence information and displays it as a QuantitativeTrack. It is implemented as a DataAdapter. It works well. My users are able to have GC content and a whole host of other simple statistics available without the need for writing lots of separate plugins or creating lots of static BigWig tracks.

I am working on trying to improve usability on this plugin so that I can release it publicly.

The Basic Problem

The thing I'm currently having trouble with is that I can't seem to add custom menu items that allow the user to change the settings for the DataAdapter on the track. These things can be customized if the track is copied and then modified, but this is a pretty big hassle for my users. I would like them to be able to change simple options for the data adapter from the track menu similar to how users are allowed to change various display options.

The developer documentation says that custom menu items can be added if a custom track type is created. I have done this, and it compiles. But when I attempt to enable the track I get the error message, "Error: no containing view found." This is due to the way getTrackMenuItems is implemented. It expects to find something that is a "view model" as defined in the core types.

The Problem's Cause

I looked through the source code, and it seems like the only things that qualify as "views," are things that descend from BaseViewModel. Track configSchemas do not. They have no width field or setWidth properties. The only things I can find that add menu items in the core components are Displays.

As far as I can tell, it is currently not possible for custom tracks to add menu items at all (despite what the documentation implies). If I want to add menu items for modifying settings for my DataAdapter I have to wrap and then extend the LinearWiggleDisplay, a wholly different kind of component. This would work, but it feels very messy.

Possible Solutions

It seems like the most elegant solution is to allow all types associated with a track to add their own menu items, not just limit it to things that extend from BaseViewModel. This would make separation of responsibilities a lot clearer and would make writing plugins much more straightforward.

People wouldn't have to add things their plugins don't need just to get access to features like adding track menu items. Right now my plugin exists as a DataAdapter. I have no functional need to fork any kind of Display or even add a custom track type. I fear that if I add these things to my plugin that it will become harder to maintain in the future by increasing complexity.

[cmdcolin]

This is an interesting concept

If you wanted to do this today, you could make a custom display type that inherits from the LinearWiggleDisplay

This is done in this example plugin jbrowse-plugin-gwas

Here is a short example showing how it would add a custom menu item https://github.com/cmdcolin/jbrowse-plugin-gwas/blob/extra_item_example/src/LinearManhattanDisplay/index.ts#L94-L100

In order to pass it to the backend, it needs a little extra wiring like this e.g. I make onClick of the menu item change a value on the display model, and then pass it to renderProps https://github.com/cmdcolin/jbrowse-plugin-gwas/blob/pass_to_backend/src/LinearManhattanDisplay/index.ts#L80-L117

The renderProps will be passed in the second argument to getFeatures e.g. so you would have

getFeatures(region, args) {
     /// can use args.customValue here
}

There could possibly be some way to make this easier, as it can be kind of tedious to make a custom display type just to add a track menu item, but that is how we are able to do it "today" in the codebase :)

Also there may be a little confusion about terminology e.g. about views

Views are like the linear genome view, the circular view, etc.
The tracks opened up in the views, and are sort of the thing you see in your config file
A display is a logic that allows a track to be "displayed" in a particular view, so there is the LinearWiggleDisplay to display a QuantiativeTrack in a LinearGenomeView

[jjrozewicki]

Thank you for your reply. I will move forward with implementing my menu items as you have suggested.

However, until the code base has changed, it seems like the documentation should be modified to reflect that adding track menu items requires a custom display type. This page (https://jbrowse.org/jb2/docs/developer_guide#adding-menu-items-to-a-custom-track) says menu items can be added with a custom track, but that appears to not be the case. It does not mention custom display at all. This mismatch caused me to waste some time because I assumed I must be doing something wrong.

On your final point about terminology, I understand the public terminology and documentation refers to things a certain way, but I was referring to things in the same way the code of JBrowse2 refers to them. The function that causes me to not be able to add track menu items to a custom track is called IsModelView.

When you grep the code you quickly find that the only things that cause this function to return true are what are things called "displays," which typically extend from the base type in the core package. This is what caused me to ask the question of whether or not a custom display type was required in order to add track menu items. The documentation said I needed a "custom track." The error message when I did that said I needed a "view." It turned out when I analyzed the code that I actually needed a "display."

I was not referring to "views," as described in the user-facing documentation. I was referring to "views" as the internal type system in the current code base describes them. This is also something it might help to clarify in the future by changing the terminology inside the code because the error message, "no containing view found," becomes a red herring. That error message says, "view," but what it's really complaining about is that your object doesn't extend from ModelView (or implement width/setWidth).

[jjrozewicki]

I followed the example as laid out in this reply:
#2006 (comment)

I was able to replace the menu items from the LinearWiggleDisplay, but when I attempted to use the pattern the LinearWiggleDisplay itself uses (also recommended in the Developer Guide) to add additional menu items to the existing ones it did not work. My modification to the example code is:

.views(self => {                                                                 
      const { trackMenuItems } = self;                                               
      console.log(trackMenuItems);                                                   
      return {                                                                       
        get trackMenuItems() {                                                       
          const new_menu_items = [                                                   
            {                                                                        
              label: "Basic extra item",                                             
            },                                                                       
          ];                                                                         
          return [...trackMenuItems, ...new_menu_items];                             
        },                                                                           
      };                                                                             
    });

Only the "Basic extra item" appears in the track menu. The array in the console log from trackMenuItems has no items in it. When I comment out this views section (but still extend the LinearWiggleDisplay with compose), I get the normal items that appear on every LinearWiggleDisplay. It's only when I attempt to override get trackMenuItems() in the way shown in the Developer Guide that I have this problem.

It appears something strange is happening with the state trees at run-time.

[cmdcolin]

@jjrozewicki thanks for digging into this. I think that there is indeed a mobx-state-tree behavior we might want to fix in this case

I made a PR to the mobx-state-tree repo to try to fix an example in their docs that caused some confusion in our own codebase mobxjs/mobx-state-tree#1724 basically we probably want to make the trackMenuItems a proper function rather than a "getter" to get proper behavior here

We are also creating a proposal in issue #2032 about a possible way for plugins to add things like menu items without needing to create a custom display or track type

[cmdcolin]

There might just be a waiting period before we get that fixed but just thought I'd update you on that. If you wanted to try to workaround, you can look at some other track types in our codebase that add trackmenuItems using the "composedTrackMenuItems" getter, but this is sort of an anti-pattern that grew in our codebase due to not having trackMenuItems be a proper function

[cmdcolin]

some of the concepts of the software engineering concepts behind trackMenuItems have been cleaned up in #2226

If we are interested, we also have the ability to put "extension points"

here is an example of adding to an extension point that registers extra right-click menu items https://jbrowse.org/jb2/docs/developer_guide#adding-track-context-menu-items

if interested in more details let me know. really appreciate these threads, they really helped to drive the progress on this!