os-js/OS.js

Adding context menu to file manager according to packages installed on OSjs.

Closed this issue ยท 10 comments

Hi,
Some written application like Virus Scan do checking on files. I think it should be better to have dynamic context menu in filemanager. Consider bellow scenario:
We wrote a cloud based virus scan package that checks files. for this, we drag and drop file to virus scan. If there be a capability in filemanager that gets custom context menu from various packages, that could runs specific functionality, I think it could be awesome.
certainly it needs to change the way filemanager builds context menu and also packages must has option to register new context menu for filemanager.

I can probably add a way to define this in src/client/config.js as:

{
  filemanager: {
    contextmenu: [
      {
        type: 'file' | 'dir' | 'special',
        event: 'myEventName',
        label: 'Something' // Automatically translated if defined in either filemanger locales or core cocales
      }
    ]
  }
}

You can then register the following event anywhere:

core.on('osjs/filemanager:contextmenu:myEventName', (file) => {
  // Do what you want here
})

Does this sound good ?

custom context menu from various packages

Since packages are loaded dynamically there's no way to have a package do something in another package without launching both first.

So it would have to be something like I mentioned in my previous comment. However, it's possible to include a configuration bootstrap in your packages to make it sort of possible, but statically.

Example:

// my-package/config.js
export function register(core) {
  core.on('osjs/filemanager:contextmenu:myEventName', (file) => {
    // Do what you want here
    // Like launch the app you want with this file, or send the file to an already open app
  })
}

export const config = {
  filemanager: {
    contextmenu: []
  }
}
// OS.js/src/client/config.js
import { config as myPackageConfig } from 'my-package/config.js'
import { config as someOtherPackageConfig } from 'other-package/config.js'
import deepmerge from 'deepmerge'

const config = deepmerge({
  // ... standard config here...
}, myPackageConfig, someOtherPackageConfig)

export default config
// OS.js/src/client/index.js
import { register as registerMyPackage } from 'my-package/config.js'

const init = () => {
  // ...
  registerMyPackage(osjs)
  // ...
}

Come to think of it, it would be possible to add the actual configuration section into metadata.json of a package, but the actual event has to be registered at some point in order for it to take effect.

I can probably add a way to define this in src/client/config.js as:

{
  filemanager: {
    contextmenu: [
      {
        type: 'file' | 'dir' | 'special',
        event: 'myEventName',
        label: 'Something' // Automatically translated if defined in either filemanger locales or core cocales
      }
    ]
  }
}

You can then register the following event anywhere:

core.on('osjs/filemanager:contextmenu:myEventName', (file) => {
  // Do what you want here
})

Does this sound good ?

I think what you mentioned here could be a good solution. Having a registry for the file-manager context menu, a third party application (e.g. virus scan, ...) can register itself.

Since packages are loaded dynamically there's no way to have a package do something in another package without launching both first.

Hi, Thank you for your awesome work.

I'm getting started working on OS.js and my comment may not be valid.

IMO it's better to keep packages portable and keep all of their configs in the package. Also, it may be useful to be able to customize context-menu items dynamically. For example, think of an application like WinRAR. It needs to add an item in the context-menu (say "Extract") only for compressed files.

Is there a way for FileManager to keep its context-menu items somewhere in the core so that every other package can customize it dynamically?

// my-package/config.js
export function register(core) {
  core.on('osjs/filemanager:contextmenu:beforerender', (file, contextMenuItems) => {
    // Do what you want here
    // Like adding/removing items to/from contextMenuItems and pass it to the next
    return contextMenuItems
  })
}

This will register the passed function in the core where context-menu items are living. When FileManager wants to render the context-menu, it needs to read it from the thing in the core, and it should run all of the registered functions to calculate the final contextMenuItems. Does it make any sense?

Does this sound good ?

It sounds good. But even could be seen more general, not only for filemanager package but also for every packages. for example a spread sheet package could has context menu to translate sentences.

So it would have to be something like I mentioned in my previous comment. However, it's possible to include a configuration bootstrap in your packages to make it sort of possible, but statically.

I think it has an advantage if a package developer doesn't need to have registers packages to src/client/config.js manualy, it is clear there is requirement to have dynamic config discovery mechanism to register each package config automatically. I think by this way a developer just needs to know how develops a package and make his own config.

This will register the passed function in the core where context-menu items are living. When FileManager wants to render the context-menu, it needs to read it from the thing in the core, and it should run all of the registered functions to calculate the final contextMenuItems. Does it make any sense?

Makes sense :)

it is clear there is requirement to have dynamic config discovery mechanism to register each package config automatically. I think by this way a developer just needs to know how develops a package and make his own config.

I like @MoradiDavijani's idea because it removes the need for any configuration manipulation (and therefore no merging required).

So here's a brain dump...

Doing that via discovery or dynamic loading has a couple of disadvantages:

  1. No longer a part of the core bundle, which makes for a bit more overhead in overall loading size
  2. Would have to be loaded upon startup, which increases loading time. And since it's not part of the core bundle there's 1 file per package that would have to be loaded.
    • Unless if it were possible to do this from the standard "main.js" file where you register the application. But this would require loading the package bundle upon startup. Which takes longer to load than a separate file just for the contextmenu registration.

But also comes with some advantages:

  1. No more configuration manipulation
  2. Does not require npm run build to take effect

Maybe the ease of use here outweighs the disadvantages ?! I'm usually a bit concerned about loading extra assets upon startup, but if this was split up into a separate file it would not be that much of an impact, especially seeing that this would be mostly used for third party stuff. It could also be loaded in the background.


But even could be seen more general

Yeah, this is probably a good idea. And maybe make not even just for context menu stuff, but arbitrary things. I'm thinking something like this:

// Service contract
const middlewareContract = {
  add: (group, cb) => {},
  get: (group) => {}
}

core.singleton('osjs/middleware', middlewareContract)

// Global API exposure
window.OSjs = {
  // ...
  middleware: (group, cb) => middlewareContract.add(group, cb)
}
// Package metadata
{
  // This would have to be added to the webpack config as well
  "preload": [
    {
      "background": true,
      "filename": "middleware.js"
    }
  ]
}
// Package middleware definition "middleware.js"
import osjs from 'osjs'

osjs.middleware('osjs/filemanager:contextmenu:edit', (({ file }) => {
  if (file.isDir) {
    return []
  }

  return [
    {
      label: 'Custom entry',
      onclick: () => {}
    }
  ]
})
// Filemanager implementation
const createAdditionalEditContextMenu = async (core, file) => {
  const middleware = core.make('osjs/middleware').get('osjs/filemanager:contextmenu:edit')
  const menus = asyncMap(middleware, m => m({ file }))
  return [].concat(...menus)
}

I'm going to prematurely close this issue even though it was linked in a PR.

This has been super-seeded by os-js/osjs-client#144

This has been released with @osjs/client@3.5.0 and @osjs/filemanager-application@1.5.0 ๐ŸŽ‰

Thank you guys so much ๐Ÿฅ‡