os-js / osjs-gui

OS.js GUI Module
https://manual.os-js.org/v3/
Other
18 stars 10 forks source link

Feat/contextmenu add shortcut support #39

Closed barsvinicius closed 1 year ago

barsvinicius commented 2 years ago

What changed

I added a new feature that allows the user to set a "shortcut" string in the context menu. This is particularly useful when the user builds an application with lots of tool-like functionalities and relies on keyboard shortcuts. This new feature allows the user to expose the shortcuts in the context menus. The screenshot below shows the feature in action:

Screenshot 2021-11-06 at 04 22 07

I also changed the menu caret for a more visually pleasing arrow.

In order to add a shortcut to the menu, add the shortcut property to the menu definition, as a string. Example:

this.core.make('osjs/contextmenu').show({
  position: ev,
  menu: [{
    label: "Save File",
    shortcut: "CTRL+S"
  }]
});

Further comments

andersevenrud commented 2 years ago

Sorry for the late feedback here. This sort of slipped my mind :sweat_smile:

This is good stuff! The only thing I would really love to see added to this is the actual binding of the shortcut keys.

There are utilities in the client to achieve this, but they are currently integrated into the global keybindings that can be added in configuration file(s). If these were exposed, this library could have an abstraction for ephemeral keybindings that could be read directly from this new shortcut property (and it already speaks the same language, ex. ctrl+alt+a)

barsvinicius commented 2 years ago

@andersevenrud, I agree that the key bindings would be great. The reason why I decided not to touch this now is precisely because we can use the client utility to do the bindings independently, so it's already achievable. Would that be okay?

andersevenrud commented 2 years ago

Currently the client integration does not support adding shortcuts on runtime. Would be very sweet to get that in so it could be integrated here.

I'm thinking maybe something like this:

// A window method to register keybindings
win.registerKeybinding({
  name: 'search',
  keybinding: 'ALT+F',
  callback: () => {}
})

// Then a simple utility to convert create options that can be passed along to the contextmenu 
const createContextMenuOptions = name => {
  const found = win.getKeybinding('search')
  if (found) {
    return {
      shortcut: found.keybinding, // Might make sense to rename this to keybinding as well
      onclick: found.callback,
    }
  }
  return {}
}

// An example of use
const createMyContextMenu = (position) => core.make('osjs/contextmenu', {
  position,
  menu: [{
    label: 'Search for something',
    ...createContextMenuOptions('search')
  }]
});
ajmeese7 commented 1 year ago

@andersevenrud how do you want to proceed on this now?

andersevenrud commented 1 year ago

Not really sure @ajmeese7 . Feel free to look at it if you have some time :)

barsvinicius commented 1 year ago

I will take another look at it, just give me a week or so. I have been pretty busy during the last month this.

andersevenrud commented 1 year ago

I have been pretty busy during the last month this.

This also applies to me :) So I have been unable to follow up activity in this project in general.

Take your time @barsvinicius . And if you find yourself too busy, there's enough information in this PR for someone else to pick it up and finish, so no worries.