sprotty icon indicating copy to clipboard operation
sprotty copied to clipboard

Alternative context menu handling

Open avshenuk opened this issue 5 years ago • 1 comments

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:

  1. 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.
  2. 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.
  3. 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;
    }
}

avshenuk avatar Nov 21 '20 13:11 avshenuk

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.

planger avatar Feb 24 '21 17:02 planger

should be done and can be closed

FelixJoehnk avatar Nov 14 '22 16:11 FelixJoehnk