feat(tools): custom keybinds and layer independent tool keybinds #464
Conversation
|
|
||
| declare var NEUROGLANCER_DEFAULT_STATE_FRAGMENT: string|undefined; | ||
|
|
||
| type CustomBinding = { | ||
| layer: string, tool: string, protocol?: string, |
There was a problem hiding this comment.
tool should be a json object, not just a string, as some tools support options
| }); | ||
| } | ||
|
|
||
| const nameToLayer: {[key: string]: UserLayerConstructor|undefined} = {}; |
There was a problem hiding this comment.
This already exists as layerTypes in neuroglancer/layer.ts
|
@jbms, is it possible to get this merged? We need this feature in ng for proofreading H01 with CAVE. |
chrisj
force-pushed
the
cj-custom-keybinds
branch
from
9428faa to
bc4f166
Compare
|
Updated this pull request with the ability to unbind existing keybinds with a false value and also with the latest code formatting. Side note: I am curious if you have vs code (if that is what you use) config set up to automatically do correct imports. My current issue is that I have to manually add the # before src, fix import order, and split out type only imports. Example config {
"keym": {
"layer": "segmentation",
"tool": "grapheneMergeSegments",
"provider": "graphene"
},
"keyc": {
"layer": "segmentation",
"tool": "grapheneMulticutSegments",
"provider": "graphene"
},
"keyf": {
"layer": "segmentation",
"tool": "grapheneFindPath",
"provider": "graphene"
},
"keyx": false
} |
| [key: string]: CustomToolBinding | string | boolean; | ||
| }; | ||
|
|
||
| declare const CUSTOM_BINDINGS: CustomBindings | undefined; |
There was a problem hiding this comment.
Please rename to NEUROGLANCER_CUSTOM_INPUT_BINDINGS
| } else { | ||
| viewer.inputEventBindings.global.set(key, `tool-${val.tool}`); | ||
| const layerConstructor = layerTypes.get(val.layer); | ||
| if (layerConstructor) { |
There was a problem hiding this comment.
Probably should throw or log an error if layerConstructor not found to avoid silently ignoring invalid input. Since this is a build time option better to ensure it is correct.
| if (hasCustomBindings) { | ||
| for (const [key, val] of Object.entries(CUSTOM_BINDINGS!)) { | ||
| if (typeof val === "string") { | ||
| viewer.inputEventBindings.global.set(key, val); |
There was a problem hiding this comment.
Note that in addition to the global map there are several others that are relevant in default_input_event_bindings.ts --- in particular the default slice view and perspective view bindings. Perhaps this should account for that?
| const protocol = viewer.dataSourceProvider.getProvider( | ||
| dataSource.spec.url, | ||
| )[2]; | ||
| if (protocol === desiredProvider) { |
There was a problem hiding this comment.
With the kvstore changes getProvider is not exactly the right method anymore. Perhaps just have a regexp that matches on the URL?
| activate(key: string): Borrowed<Tool> | undefined { | ||
| const tool = this.get(key); | ||
| activate(key: string, tool?: Tool<object>): Borrowed<Tool> | undefined { | ||
| tool = tool || this.get(key); |
There was a problem hiding this comment.
Please use tool ?? this.get(key) instead --- not necessary here but avoids thinking about javascript truthyness complications
| if (tool === undefined) { | ||
| this.deactivate_(); | ||
| return; | ||
| } | ||
| this.debounceDeactivate.cancel(); | ||
| const activeTool = this.activeTool_; | ||
| if (tool === activeTool?.tool) { | ||
| if (tool.toJSON() === activeTool?.tool.toJSON()) { |
There was a problem hiding this comment.
We can't compare the json like this because it can be an object rather than a plain string. Instead we need to compare the serialized json. However, it would be better if we can avoid having to insert the json comparison here, and instead just ensure that the same tool object is passed. Is your logic involving previousTool not sufficient?
|
Regarding your question about the imports, in case you haven't already figured it out --- npm run lint:fix fixes some things. I don't use vscode --- but the typescript language server does have some options for configuring how it spells imports that it generates. Unfortunately as far as I am aware there is no standard way to specify those configuration options --- instead they need to be specified in some editor-specific way. If there is a way to configure it for vscode with a config file I'm happy to include such a configuration file in this repo. |
|
|
||
| declare let NEUROGLANCER_DEFAULT_STATE_FRAGMENT: string | undefined; | ||
|
|
||
| type CustomToolBinding = { | ||
| layer: string; |
There was a problem hiding this comment.
rename to layerType for consistency with the tool palette queries.
🔥 Preview 🔥: https://neuroglancer--pr464-jqtohdii.web.app
Updated at Thu Feb 22 2024 17:27:49 GMT+0000 (Coordinated Universal Time) for commit bc4f166
Expires: Invalid Date