[nhaagen]

following #7045, this adds Prompts to DataTable's Actions.

[nhaagen]

this PR includes #7045

[nhaagen]

There is a change in the interface of Prompt - it now is constructed with an URLBuilder instead of an URI.
This allows us to add parameters to an otherwise fixed URL.
A Prompt should always point to its endpoint; with its use in the table, it is not economical to add a new Prompt for every
action per row. So, with this change, we can still use distinct prompts to perform actions and add a parameter, in this case the row-id.

[klees]

Hi @thibsy and @Amstutz,

please review this!

Thanks!

[thibsy]

Hi @nhaagen,

Thanks a lot for your contribution to the UI components!

About the concrete changes, please answer the following questions:

• Async Table\Action\Action: what is the advantage of accepting a Prompt in the constructor vs. simply rendering the Prompt internally, when an action is considered async? Could we not leverage the URI parameter of Prompt::getShowSignal() to achieve the same result?

Kind regards,

@Amstutz and @thibsy

[nhaagen]

Hi @Amstutz & @thibsy ,

we actually do render a single default prompt as template and alter its target in the JS.

The reason for using a template is obvious: rendering a prompt for every action and row is way too much.

The construction of an action with prompt actually strengthens the prompt's semantic:

A Prompt points to an endpoint, usually a component, and thus "lives" in the component's domain.
It requires one or many states as a counterpart, and probably some action in between.
Constructing the prompt with a populated URLBuilder from that domain pins the prompt to its component with all needed params in the url - as we feel is the correct way of using a prompt.

There still is need for a "moving" parameter in order to point to the record(s), and that's provided via a token for that exact prompt/URLBuilder.

E.g., we might then write code like this at the component:

function getUserRemovalPrompt(): array
{
	//[....]
	return [$prompt, $token];
}

and use it in the table like this:

$f->table()->action()->multi('remove user', ...$component->getUserRemovalPrompt());

Best regards,
Nils

[thibsy]

Hi @nhaagen,

Thx for the explanation and sorry for the delay here.

When reviewing this PR I had a hard time to understand why we need a C\Prompt\Prompt in order to create a C\Table\Action\Action. If I understand correctly, the prompt will only be used to communicate to the action that it should be called asynchronously (by using the prompt template for C\Table\Data). The rest of the prompt is simply discarded. IMO we should either fully embrace the prompt and make the action a composite UI component, which actually renders every prompt, or we should leave the action as it is because we already have a way to determine if its asynchronous - and the prompt would then be misleading.

Rendering the internal prompt inside the I\Table\Renderer is also problematic at the moment, because you rely on the request being provided, which is not always the case. Embracing the prompt in actions would solve this problem while consumers would also benefit, because they could respond to the prompts signals this way.

Let me know how you want to proceed.

Kind regards,
@thibsy

[thibsy]

@nhaagen I assign this back to you. Feel free to reassign once we can proceed.

[oliversamoila]

Hello @nhaagen,
can you give me a status and outlook on this PR?
UI changes of all old tables in LOM Editor are waiting for it.
Many thanks to you

[thibsy]

@nhaagen is this ready for review?

[klees]

Hello everyone,

I have discussed this with @thibsy today, and I think I now understood. The core of the question is: We already have a Prompt in the Action, and Actions using Prompts (instead of URLBuilders) would be the desired state of the art there. Instead of fully embracing this, we instead deconstruct the Prompt into a URLBuilder (which might discard information at some point) only to reconstruct a Prompt later on. We should turn this the other way round: Make Prompt the real content of the Action (instead of URLBuilder) and bend the deprecated URLBuilder variant to that form.

The symptom of this is

protected URLBuilder | Prompt $url_builder,

in the constructor of Action. This should rather be:

protected Prompt |  URLBuilder  $prompt,

as this is what we actually want to have.

This makes a lot of sense to me and I kindly ask @nhaagen to adjust the code accordingly. I also expect that the code might become smaller and simpler then...

Kind regards!

[nhaagen]

Actions may now be constructed with Prompt and URLBuilder; withAsync (and the according Modals) have fallen.
All Prompts added via Actions are rendered (not per row, but per action) and triggered with their respective signals.

[thibsy]

Thx a lot @nhaagen for fully embracing the prompt. This looks a lot more straightforward IMO =).

However, you remove the withAsync() option completely now, which leaves a lot of usages not attended to in the latest trunk. I approve of the changes but cannot merge this until usages are either amended or the option is added as a transitional measure again (with deprecation). Therefore, please

• Action::withAsync(): either amend all current usages of this method or introduce it again as a transitional measure.
• Conflicts: rebase this onto the latest trunk

Kind regards,
@thibsy

[nhaagen]

I was so sure I saw a deprecation-note for 11 on withAsync - but that was probably from myself in a former version of this 🫣
a342df8#diff-2722f6d106071a286e67cbaa30002f18c4b83c5675194c206d2fe98ea703ce33R71-R73

[nhaagen]

• Action::withAsync(): either amend all current usages of this method or introduce it again as a transitional measure.
• Conflicts: rebase this onto the latest trunk

[dsstrassner]

Hi @oliversamoila, I think you should discuss with @thibsy how to proceed in the future with this PR 🙈

[thibsy]

Superseded by #12389