sprotty
sprotty copied to clipboard
Alternative context menu handling
The current logic of context menu handling has certain drawbacks:
- whenever you right-click on an empty area or a new node - it doesn't "deselect" old items same way as it does when you left-click there. And since we use the selected items as the "context" of the menu - you might get undesired results.
- the temporary select is not rendered right away for some reason - it only shows up after you do a few mouse movements. This probably has something to do with the fact that it doesn't go via the command stack but I'm not sure.
- the revert of the temp selection is not effective as most of the time you need to left-click somewhere in order to close the context menu and that triggers selection event on its own.
So I suggest an alternative logic by utilizing somewhat similar events issued by the selection listener:
- If you have something selected already and you right-click on a node FROM that selection - just do nothing and show the context menu. Those selected nodes are your context.
- If you have something selected already and you right-click on an empty area or a node OUTSIDE that selection - deselect the old nodes AND select the new node (if present). Most of the time that right click means that you don't care about the previous selection - if you do, you can always "Ctrl/Cmd select" all necessary nodes first.
- Somewhat debatable: always switch off editing of SRoutableElements whenever you right-click.
A suggestion code that worked quite well for me:
export class ContextMenuMouseListener extends MouseListener {
mouseDown(target: SModelElement, event: MouseEvent): (Action|Promise<Action>)[] {
const result = [];
if (event.button === 2) {
const mousePosition = {x: event.x, y: event.y};
const selectableTarget = findParentByFeature(target, isSelectable);
const deselect: SModelElement[] = toArray(target.root.index.all()
.filter(element => isSelected(element)
&& !(selectableTarget instanceof SRoutingHandle && element === selectableTarget.parent as SModelElement)));
if (selectableTarget) {
if (!selectableTarget.selected) {
result.push(new SelectAction([selectableTarget.id], deselect.map(e => e.id)));
result.push(new BringToFrontAction([selectableTarget.id]));
}
} else {
result.push(new SelectAction([], deselect.map(e => e.id)));
}
const routableDeselect = deselect.filter(e => e instanceof SRoutableElement).map(e => e.id);
if (routableDeselect.length > 0)
result.push(new SwitchEditModeAction([], routableDeselect));
result.push(new ContextMenuAction(mousePosition));
}
return result;
}
}
export class ContextMenuAction implements Action {
static readonly KIND = "contextMenu";
kind = ContextMenuAction.KIND;
constructor(public readonly mousePosition: {x: number, y: number}) {
}
}
@injectable()
export class ContextMenuCommand extends SystemCommand {
static readonly KIND = ContextMenuAction.KIND;
@inject(TYPES.IContextMenuServiceProvider) protected readonly contextMenuService: IContextMenuServiceProvider;
@inject(TYPES.IContextMenuProviderRegistry) protected readonly menuProvider: ContextMenuProviderRegistry;
constructor(@inject(TYPES.Action) protected readonly action: ContextMenuAction) {
super();
}
execute(context: CommandExecutionContext): CommandReturn {
this.showContextMenu(context);
return this.redo(context);
}
protected async showContextMenu(context: CommandExecutionContext): Promise<void> {
let menuService: IContextMenuService;
try {
menuService = await this.contextMenuService();
} catch (rejected) {
// IContextMenuService is not bound => do nothing
return;
}
const menuItems = await this.menuProvider.getItems(context.root, this.action.mousePosition);
menuService.show(menuItems, this.action.mousePosition, () => {
});
}
redo(context: CommandExecutionContext): CommandReturn {
return context.root;
}
undo(context: CommandExecutionContext): CommandReturn {
return context.root;
}
}
Imho this is a good proposal! Would you be able to open a PR with that proposal?
In a project on top of Sprotty, we have somewhat similar code to adapt Sprotty's default selection behavior for right-clicking. So I guess it makes sense to adapt the behavior in Sprotty too.
should be done and can be closed