feat(actions): standardize contexts

[skjnldsv]

Following discussion on #1113

This would be a breaking change unless we add a conversion layer to support the old legacy way.
BUT, the cleanest way, imho, is to quickly update server and communicate to devs asap

// Before
action.exec(node, view, '/path/to/node')

// After
action.exec({
	nodes: [node],
	view: View,
	folder: Folder,
	content: Node[],
})

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.74%. Comparing base (fd3a895) to head (d9c2e53).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1124   +/-   ##
=======================================
  Coverage   90.74%   90.74%           
=======================================
  Files          23       23           
  Lines         670      670           
  Branches      182      182           
=======================================
  Hits          608      608           
  Misses         51       51           
  Partials       11       11           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[skjnldsv]

Using 1 or 2 parameters before a context object in a function declaration does not seem that bad to me IMHO, any reason why we should never do this @skjnldsv?

Because it just looks weird to me.
It's like mixing two syntaxes that have nothing in common, like spaces and tabs 😁

From a clean declaration perspective, I would agree with
function(path, options = {})
function(path, folder, view...)
function(options = {})

But mixing something like function(path, view, dir, options = {}) seems incoherent to me.
To prove my point, and assuming there is no issue with mixing both, then why do this feels worse? function(options = {}, path, dir) or even worse: function(path, options = {}, view, dir) 🙈

But more importantly, I like to be consistent across our APIs here, and considering we have multiple APIs that do similar approaches, like FileAction, NewFileMenu, Views..., I think we should stick to one type of pattern here. That way it's easy to know what to expect when you work with the @nextcloud/files library.

This is why I was really against #1113
PS: #1135

[skjnldsv]

Done @susnux !
Updated 🚀

[skjnldsv]

@susnux I'm second guessing myself here, do you think we also need the full context here?
nodes: Node[], view: View, folder: Folder, content: Node[],

Or would only nodes: Node[], view: View should be enough ?

[skjnldsv]

I think we could probably stick closer to the original implementation

export type ActionContextBase = {
	nodes: Node[],
	view: View,
}

export type ActionContext = {
	folder: Folder,
	content: Node[],
} & ActionContextBase

export type ActionContextSingle = {
	nodes: [Node],
} & ActionContext

export type ViewActionContext = {
	view: View,
	folder: Folder,
}

export interface FileActionData {
	/** Unique ID */
	id: string
	/** Translatable string displayed in the menu */
	displayName: (context: ActionContextBase) => string
	/** Translatable title for of the action */
	title?: (context: ActionContextBase) => string
	/** Svg as inline string. <svg><path fill="..." /></svg> */
	iconSvgInline: (context: ActionContextBase) => string
	/** Condition wether this action is shown or not */
	enabled?: (context: ActionContextBase) => boolean

	/**
	 * Function executed on single file action
	 * @return true if the action was executed successfully,
	 * false otherwise and null if the action is silent/undefined.
	 * @throws Error if the action failed
	 */
	exec: (context: ActionContextSingle) => Promise<boolean|null>,
	/**
	 * Function executed on multiple files action
	 * @return true if the action was executed successfully,
	 * false otherwise and null if the action is silent/undefined.
	 * @throws Error if the action failed
	 */
	execBatch?: (context: ActionContext) => Promise<(boolean|null)[]>

[susnux]

Especially "enabled" should also have the folder, because some actions only work in specific parent folders

[skjnldsv]

ok, for enabled. But the other might not need it. Wdyt ?