Skip to content

Replace contextual menu on the bluetooth device running item by a menu#43

Merged
HaudinFlorence merged 2 commits intoQuantStack:mainfrom
HaudinFlorence:replace_contextual_menu_on_the_bluetooth_device_running_items_by_a_menu
Apr 3, 2025
Merged

Replace contextual menu on the bluetooth device running item by a menu#43
HaudinFlorence merged 2 commits intoQuantStack:mainfrom
HaudinFlorence:replace_contextual_menu_on_the_bluetooth_device_running_items_by_a_menu

Conversation

@HaudinFlorence
Copy link
Copy Markdown
Member

@HaudinFlorence HaudinFlorence commented Apr 2, 2025

Fixes sub-issue 12 of #7

@HaudinFlorence HaudinFlorence force-pushed the replace_contextual_menu_on_the_bluetooth_device_running_items_by_a_menu branch from fb77013 to 4fb07e4 Compare April 2, 2025 09:55
this.device = device;
this.commands = new CommandRegistry();
this.commands.addCommand(disconnectMoveHub, {
const commands = new CommandRegistry()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In extension code, you should never need to instantiate a CommandRegistry. The only command registry that should exist in a given Jupyter front-end should be the one that is instantiated by the application itself, i.e., the JupyterLab app.commands.

Comment on lines +34 to +39
const commandList = commands.listCommands();
commandList.map((command: string) => {
if (command.includes('bluetooth-manager') && !command.includes("disconnect")) {
menu.addItem({ command: command})
}
})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of searching through the command registry for commands that match a certain name, it would be more flexible and less risky (because you can't protect against command names that don't follow this convention) to all the BluetoothManager.Device instance that you have a reference to here this._device and create a public attribute on it, i.e. this._device.contextCommands or something like this and to pass the deviceID like you do in the line above.

@HaudinFlorence
Copy link
Copy Markdown
Member Author

HaudinFlorence commented Apr 2, 2025

@afshin Thanks for the review. The comments have been taken into account in commit 1d05396

…ands attribute to the BluetoothManager.Device class.

Move the connectMoveHub and disconnectMoveHub commands from the ConnectionStatus component to the index.
@HaudinFlorence HaudinFlorence force-pushed the replace_contextual_menu_on_the_bluetooth_device_running_items_by_a_menu branch from ac19dde to 1d05396 Compare April 2, 2025 22:16
@HaudinFlorence HaudinFlorence merged commit 548be2b into QuantStack:main Apr 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants