Need smarter contextmenu items

[telamonian]

People working on the context menu of JupyterLab have run into various problems (see jupyterlab/jupyterlab#4529 for a good overview). After spending a while thinking about it, I believe that all of the issues can be resolved by making the contextmenu items in @phorsphor/widgets a little bit smarter/more aware of the circumstances under which they were invoked. Specifically, I think that two changes are required:

• There needs to be a way to pass information contained in the target of a contextmenu event (ie the node that the user actually right-clicked) on to the command of a contextmenu item.

• There needs to be a way to decide whether to hide/show a contextmenu item based on the result of a callback that takes the contextmenu event target as an argument. This would be in addition to (or possibly instead of) the existing selector property.

[telamonian]

I've already written a PR (#355) addressing the first bullet point. It adds a passDataset property to contextmenu items. If this flag is set, then the key-value pairs of a node's dataset will be added to the properties of the args of any matching item.

The use case for this PR is the tab focus issue described in jupyterlab/jupyterlab#4945. In short, the commands of the contextmenu of each document-tab need to be aware of the path associated with the tab that was actually right-clicked on. The PR would allow for a nice simple fix, by just adding a path to the dataset attached to each document-tab.

[blink1073]

For 2, you can use CSS dataset selectors in the registered context menu item.

[telamonian]

Hmmm, you're definitely not wrong, since combinators and pseudo-classes do allow for the composition of selectors with complex criteria (though, full disclosure, I actually didn't know that until your comment spurred me to find a decent resource on the topic). However, I still think there'll be complex cases where it'll be easier to write a selector callback function than to compose a long and convoluted CSS selector.

The use case I have in mind is to migrate the Jupyterlab filebrowser context menu over to the application-wide context menu, as per @ian-r-rose in jupyterlab/jupyterlab#4529.

[blink1073]

Hmm, as long as the context menu handler is scoped to an appropriate selector (not body), I think it is fair to give it a chance to show or not based on the current event target (performance-wise).

[sccolbert]

I agree with the thesis of this thread. We've known this for a while. So far nothing has hit me as the "right" way to provide that information. I agree with @blink1073 that CSS selectors should be enough to control visibility, provided you tag your node with enough information.

My current thinking is I'd like to just find a clean way to pass the DOM node to command which is executed, but there may be some unintended consequences to that, such as what to pass when the command is invoked directly?

[sccolbert]

At this point we are not getting rid of selectors, since that would be a breaking change.

[telamonian]

but there may be some unintended consequences to that, such as what to pass when the command is invoked directly??

My thoughts exactly. When I set out to write #355 I did originally intended to just have it pass the target node to a command, but then I realized that would make it awkward to write commands that could both be used in contextmenu items and invoked directly. The design I settled on is intended to make invocation simple, since this way the names of the dataset properties you'd set when invoking a command via a context menu would be the same as the names of the args you'd pass when invoking the command directly.

Still, if you think it's better to pass the whole node, I guess it wouldn't be that big a deal? You would just need to test for/branch on the presence of an optional node arg (or static property, or whatever) within the body of relevant commands.

[hadim]

What is the status of this issue? I am looking for a way to select only certain file according to their extension ('.zip', 'tar,bz2', etc). See jupyterlab-contrib/jupyter-archive#19 for details.

In CSS you can add multiple selectors with a comma:

const selectorOnlyArchive = '.jp-DirListing-item[title$=".zip"], .jp-DirListing-item[title$=".tgz"]';

But it does not seem to work in Jupyter. And actually the documentation is pretty clear about it: https://phosphorjs.github.io/phosphor/api/widgets/interfaces/contextmenu.iitemoptions.html

The selector must not contain commas.

So is there is a way with the current API to achieve this kind of selection only with the selector attribute?

[jasongrout]

Are those selectors mutually exclusive? If so, can you add one item for each selector?

[hadim]

Indeed this is what I ended up doing. See jupyterlab-contrib/jupyter-archive#19

[telamonian]

provided you tag your node with enough information.

@hadim Following this piece of advice from @sccolbert was ultimately how I ended up fixing all of my selector issues. By being careful with classnames and dataset contents, I was able to solve ~90% of my contextmenu item selector issues.

The trickiest cases all involved situations in which I needed to distinguish between two different areas, but for which there were no mutually exclusive selectors. For these cases, I either rearranged my existing DOM, or I added an extra invisible div to help distinguish one area from another.