[ItemGroup] Add New Object Action for Item Groups

[lukas-heinrich]

Hi everyone,

In this PR, @matheuszych and I are providing an implementation of the feature “Add New Object Action for Item Groups”.

During the implementation, we ran into difficulties integrating the header action for item groups. That is why this PR depends on a previous fix we provided separately in #11548.

Best
@lukas-heinrich

/cc @thojou

[kergomard]

Thank you very much for the PR @lukas-heinrich

Please also see my comment here: #11548 .

Thank you for the improvements in the DocBlocks and the moving of the funnily placed continue.

I feel unable to provide you with a lot of feedback in the short and middle term, so here are just a few quick things, that I can already tell you:

• I cannot allow you to set a Modal on AddNewItemGUI this is not how this should work. The class is built as an immutable (even if it is not properly declared as such yet).
• I do not like the construction of the modal in the constructor and the assignment to a class-property. This has to go.
• Please do not change line breaks, especially not in the constructor.

I could imagine that you rename the function AddNewItemGUI::render() to AddNewItemGUI::renderToolbarAction() and that you then add a second function AddNewItemGUI::getModal() that just builds the modal and returns it (AddNewItemGUI::renderToolbarAction() could then internally use this function to build the modal). That's right now the best I can come up with. The responsibility for any caching needs to be on your side, but I do not think you need any.

I hope this makes at least some sense.

Best,
@kergomard

[lukas-heinrich]

Hi @kergomard ,

Thank you very much for your feedback on these changes. I have removed the dependencies from the AddNewItemGUI class and applied the requested changes to the code. In my opinion, the structure is now much better than before.

Best
@lukas-heinrich

[alex40724]

The one-liner in the container renderer is ok as an intermediate solution for me.

[thojou]

Hi @kergomard ,

i've reworked the implementation and slightly adjusted how we interact with the modal in AddNewItemGUI. Could you please take a quick look at these changes?

https://github.com/ILIAS-eLearning/ILIAS/pull/11549/changes#diff-d609cc89b9b07919026ff78ed488d5ef5af6b8afe48fd7a538b368cd891d6745R66-R82

Essentially, I introduced a closure that allows us to perform the necessary manipulations, enabling the modal to open during rendering. This approach keeps the implementation stateless, as we avoid storing any mutable state inside the class itself.

As an alternative, I considered implementing a withOpened() (or similar) method to explicitly set the state, but I decided against it. The closure approach is more flexible and avoids over-specifying the API while still allowing additional manipulations if needed.

I also refactored this PR to remove the dependency on #11548. The feature now works without these changes. We are now relying on a redirect followed by modal rendering.

Let me know what you think. Once we’re done with the review, I’ll squash everything into a single commit.

Thanks!

[thojou]

Hey @kergomard,

As discussed, I’ve reworked the implementation again and split the render method out of AddNewItemGUI. In an additional commit 5606ef, I renamed render to renderToolbarAction.

Please let me know if you’d like any further changes. Otherwise, from our point of view, this PR is ready to go.

Kind Regards,
@thojou