Skip to content

Commit

Permalink
menu: make command-less menus more robust
Browse files Browse the repository at this point in the history
The commit updates `menus` to not fail the layout when there are no
commands attached to them, and instead log the error. Previously, if a
command was unregistered and a command referenced it the layout would
fail, and possibly also fail to start the application.

The change also includes:
- a bogus menu registration in `api-samples` to verify the feature in the future.
- an api test to verify that registration does not fail for menus with invalid commands.

Signed-off-by: vince-fugnitto <[email protected]>
  • Loading branch information
vince-fugnitto committed Aug 16, 2021
1 parent 1dbed43 commit 84bca9b
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ export class SampleMenuContribution implements MenuContribution {
});
const placeholder = new PlaceholderMenuNode([...subSubMenuPath, 'placeholder'].join('-'), 'Placeholder', { order: '0' });
menus.registerMenuNode(subSubMenuPath, placeholder);

/**
* Register an action menu with an invalid command (un-registered and without a label) in order
* to determine that menus and the layout does not break on startup.
*/
menus.registerMenuAction(subMenuPath, { commandId: 'invalid-command' });
}

}
Expand Down
4 changes: 4 additions & 0 deletions examples/api-tests/src/menus.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,8 @@ describe('Menus', function () {
assert.isTrue(access2.disposed);
});

it('should not fail to register a menu with an invalid command', () => {
assert.doesNotThrow(() => menus.registerMenuAction(['test-menu-path'], { commandId: 'invalid-command', label: 'invalid command' }), 'should not throw.');
});

});
3 changes: 2 additions & 1 deletion packages/core/src/common/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,8 @@ export class ActionMenuNode implements MenuNode {
}
const cmd = this.commands.getCommand(this.action.commandId);
if (!cmd) {
throw new Error(`A command with id '${this.action.commandId}' does not exist.`);
console.debug(`No label for action menu node: No command "${this.action.commandId}" exists.`);
return '';
}
return cmd.label || cmd.id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ export class ElectronMainMenuFactory {

// That is only a sanity check at application startup.
if (!this.commandRegistry.getCommand(commandId)) {
throw new Error(`Unknown command with ID: ${commandId}.`);
console.debug(`Skipping menu item with missing command: "${commandId}".`);
continue;
}

if (!this.commandRegistry.isVisible(commandId, ...args)
Expand Down

0 comments on commit 84bca9b

Please sign in to comment.