#88 Add the DropdownAction

[alexandre-castelain]

Hey !

This is the first step to create the DropdownAction. Do not merge it as is.

On the theme side :

• It uses a new stimulus controller to trigger the dropdown to open/close
• For the base theme, I add a little CSS directly in the template. Not the best way to do it, but I don't want to create assets for the bundle just for that (yet).

On the PHP side :

This is the way I want to add row actions :

->addRowAction('multipleActions', DropdownActionType::class, [
    'actions' => function (RowActionBuilderInterface $builder) {
        return [
            $builder->addRowAction('update', ButtonActionType::class, [
                'href' => function (Product $product) {
                    return $this->urlGenerator->generate('app_product_update', [
                        'id' => $product->getId(),
                    ]);
                },
            ])
        ];
    },
])

As you can see, I inject a RowActionBuilderInterface in the callback to add the actions. And the DataTableBuilderInterface extends it.
I like it, because we could add a DropdownAction to a DropdownAction if we want it. It adds a lot of flexibility.

At this point, I'm not sure where I should resolve this callback. Neither if we should inject a DataTableBuilder or another class.
What do you think about it ? Any advice ?

Have a good day !
Alexandre

[Kreyu]

Hey @alexandre-castelain.

What if we could completely drop the RowActionBuilderInterface, and just use the DataTableBuilderInterface? We can use the createAction, createRowAction and createBatchAction methods from the builder. The dropdown action should not be limited to the row actions alone.

The DropdownActionType would accept a actions option with allowed types ActionBuilderInterface[] ActionBuilderInterface[], or callable:

$builder
  ->addRowAction('multipleActions', DropdownActionType::class, [
      'actions' => [
          $builder->createRowAction('update', ButtonActionType::class, [
              'href' => function (Product $product) {
                  return $this->urlGenerator->generate('app_product_update', [
                      'id' => $product->getId(),
                  ]);
              },
          ])
      },
  ])
  ->addRowAction('multipleActionsCallback', DropdownActionType::class, [
      'actions' => function (mixed $data) { // $data is row data, since its a row action
          return [
              $builder->createRowAction('update', ButtonActionType::class, [
                  'href' => function (Product $product) {
                      return $this->urlGenerator->generate('app_product_update', [
                          'id' => $product->getId(),
                      ]);
                  },
              ])
          ];
      },
  ])
;

It would work the same with addAction (global action) and addBatchAction (batch action).

At this point, I'm not sure where I should resolve this callback. Neither if we should inject a DataTableBuilder or another class.

I think, directly in the DropdownActionType, something like that ("pseudo" code - not tested of course, just an idea 😄):

final class DropdownActionType extends AbstractActionType
{
    public function buildView(ActionView $view, ActionInterface $action, array $options): void
    {
        $children = [];

        $actions = $view->vars['actions'];

        // Resolve the callable if parent is column value (the action is row action)
        if ($view->parent instanceof ColumnValueView) {
            $actions = $actions($view->parent->value);
        }

        foreach ($actions as $child) {
             if ($child instanceof ActionBuilderInterface) {
                 $child = $child->getAction();
             }

             $children[] = $child;
        }

        $view->vars = array_replace($view->vars, [
            'children' => $children,
        ]);
    }
}

And in the Twig, we could iterate through the children variable and render each action in a dropdown. And this is the hardest part for me, as we are accepting actions of any type inside the dropdown. How to render the button action, for example? Or some "weird" custom type, with fancy rendering? Maybe we should either create a second type, for example, a DropdownItemActionType, and only accept those types as children, or just nest another DropdownActionType by itself.

[alexandre-castelain]

What if we could completely drop the RowActionBuilderInterface, and just use the DataTableBuilderInterface? We can use the createAction, createRowAction and createBatchAction methods from the builder. The dropdown action should not be limited to the row actions alone.

I worked only on the row action at this time, but doing as you propose, we could do this :

->addRowAction('multipleActions', DropdownActionType::class, [
      'actions' => [
          $builder->createBatchAction('update', ButtonActionType::class, [...]
      ]
  ])

Add a batchAction in a rowAction. This doesn't make any sense to me.

Using a callback, we would be sure the builder has only the correct method to use for the current action type. For a DX point of view, it's easier if there's less options available.
Also, if someone (as I did) use the "addAction" instead of the "createAction", the action would be added not on the correct place. So we should consider this

Maybe we should either create a second type, for example, a DropdownItemActionType, and only accept those types as children, or just nest another DropdownActionType by itself.

Yeah, I like this idea.

[Kreyu]

Add a batchAction in a rowAction. This doesn't make any sense to me.
Using a callback, we would be sure the builder has only the correct method to use for the current action type. For a DX point of view, it's easier if there's less options available.

I think you're right. That could be handy for autocompletion. I still don't like the idea of requiring a callback for that, but maybe that's just me 😄 Also, remember that this also would be correct:

$builder->addRowAction('multipleActions', DropdownActionType::class, [
      'actions' => [
          $builder
              ->createAction('update', ButtonActionType::class, [...])
              ->setContext(ActionContext::Row)
      ]
])

However, you can still define a child action with any action type. We can validate this in buildView method, but I think there's no way to help with that in IDE autocompletion.

Also, we have to validate the context of each action though, probably in the buildView as well. We could also change the context of each action to the same as the parent, that would be handy, but probably too confusing in some cases 😄

Also, if someone (as I did) use the "addAction" instead of the "createAction", the action would be added not on the correct place. So we should consider this

That is true, and this will throw an exception, because the option does not allow values of type DataTableBuilderInterface (which is returned from the fluent addAction()). I don't think we can improve the DX without creating a ton of small interfaces for the data table builder. I'm worried that over time, the builders will implement many different interfaces just for the sake of autocompletion, which in my opinion is not worth it 😞

However, another idea - we can change the definitions of the actions for the key-value map, like so:

$builder->addRowAction('multipleActions', DropdownActionType::class, [
      'actions' => [
          'update' => ['label' => 'Update something'],
          'delete' => ['label' => 'Delete something'],
      ]
]);

and then, based on the context of the parent action (global, row, batch) create an action of correct type and context:

final class DropdownActionType extends AbstractActionType
{
    public function buildAction(ActionBuilderInterface $builder, array $options): void
    {
        // like in CollectionColumnType, we're saving the factory as attribute,
        // so we can access it in buildView() without unnecessary dependency injection
        // the symfony form component is also doing this in source in CollectionType
        $builder->setAttribute('action_factory', $builder->getActionFactory());
    }

    public function buildView(ActionView $view, ActionInterface $action, array $options): void
    {
        /** @var ActionFactoryInterface $actionFactory */
        $actionFactory = $action->getConfig()->getAttribute('action_factory');

        $children = [];

        $actions = $view->vars['actions'];

        // Resolve the callable if parent is column value (the action is row action)
        if ($view->parent instanceof ColumnValueView) {
            $actions = $actions($view->parent->value);
        }

        foreach ($actions as $name => $options) {
             $children[] = $actionFactory
                 ->createNamedBuilder($name, DropdownActionType::class, $options) // here we use the correct action type
                 ->setContext($action->getConfig()->getContext()) // here we use the correct context (global/row/batch)
                 ->getAction()
             ;
        }

        $view->vars = array_replace($view->vars, [
            'children' => $children,
        ]);
    }
}

I feel like we're so close to establish a solid API for the dropdown action, yet so far 😅

[Kreyu]

On the second thought, I think we are slightly... overthinking this. What if we simply create a DropdownActionType with items options, where each item contains predefined options? If you think about it, what do we need to specify each dropdown item:

• a label, obviously, to display in the dropdown;
• an url to execute the action;
• a method, for actions using something other than GET, e.g. a DELETE for entity removal;
• an array of HTML attributes for the dropdown item;

This can be handled without touching current builder API, does not have any weird quirks related to action context (is it row? is it batch?) nor action type (dropdown item is dropdown item, simple as that).

final class DropdownActionType extends AbstractActionType
{
    public function buildView(ActionView $view, ActionInterface $action, array $options): void
    {
        // resolve items, add to $view->vars
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver
            ->setDefault('items', function (OptionsResolver $itemResolver): void {
                $itemResolver
                    ->setDefaults([
                        'url' => '#',
                        'method' => 'GET',
                        'attr' => [],
                    ])
                    ->setRequired('label')
                    ->setAllowedTypes('label', 'string')
                    ->setAllowedTypes('url', 'string')
                    ->setAllowedTypes('method', ['null', 'string'])
                    ->setAllowedTypes('attr', 'array')
                  ;
            })
        ;
    }
}

Now, I don't know if this method has any drawbacks, but what if we render each item as a form - so it can do any given method? https://jsfiddle.net/oravm9bf/5/

<div class="dropdown">
    <button class="btn btn-secondary dropdown-toggle" type="button" data-bs-toggle="dropdown" aria-expanded="false">
        Dropdown button
    </button>
  
    <ul class="dropdown-menu">
        {% for item in items %}
            <li>
                {# Note: form element method is either GET or POST, other methods belong to hidden _method input #} 
                <form action="{{ url }}" method="{{ method == 'GET' ? 'GET' : 'POST' }}">
                    <input type="hidden" name="_method" value="{{ method }}">
                    {# Here we should merge "attr" with type, class and value attributes and render it #}
                    {# using "attributes" block, but for the sake of simplicity of this example, I'm skipping it #}
                    <input type="submit" class="dropdown-item" value="{{ label }}" {{ block('attributes') }} />
                </form>
            </li>
        {% endfor %}
    </ul>
</div>

This is something I was thinking about to merge built-in FormActionType into ButtonActionType, to make it more simple and easier to use. Optionally, we can check if item method option equals "GET", and then, render it as a simple anchor:

<div class="dropdown">
    <button class="btn btn-secondary dropdown-toggle" type="button" data-bs-toggle="dropdown" aria-expanded="false">
        Dropdown button
    </button>

    <ul class="dropdown-menu">
        {% for item in items %}
            <li>
                {% if method == 'GET' %}
                    <a class="dropdown-item" href="{{ url }}" {{ block('attributes') }}>{{ label }}</a>
                {% else %}
                    <form action="{{ url }}" method="POST">
                        <input type="hidden" name="_method" value="{{ method }}">
                        <input type="submit" class="dropdown-item" value="{{ label }}" {{ block('attributes') }} />
                    </form>
                {% endif %}
            </li>
        {% endfor %}
    </ul>
</div>

What do you think? I think we can skip a lot of complexity, while also giving a lot of new functionality. Easier to use (API not changed, options easily validated), and easier to maintain. You can even handle each action with JavaScript, for example, with Stimulus, by simply adding an data-action attribute.

[alexandre-castelain]

Hey,

I'll try to focus on your second proposal, because it looks promising.

On the second thought, I think we are slightly... overthinking this. What if we simply create a DropdownActionType with items options, where each item contains predefined options? If you think about it, what do we need to specify each dropdown item:
- a label, obviously, to display in the dropdown;
- an url to execute the action;
- a method, for actions using something other than GET, e.g. a DELETE for entity removal;
- an array of HTML attributes for the dropdown item;

Clearly, this approach seems much simpler. On the other hand, I also find that we lose the reuse of other actions.

For example:

• It would be interesting to put a DropdownAction inside another DropdownAction (the name would probably need to be revised), to have a multi-level action breakdown.
• In this kind of action, in the project I am working on, we do not only use links. Sometimes, we need to make an AJAX call on a route, with a confirmation popup. Sometimes, I want to open a modal to display information (a form, a partial detail of the entity, or data tables of related entities).
• I also know that these are not necessarily actions you want to add to the base bundle, but if we create our own actions, how can we add them to the DropDownAction?

I think if we go with this "simplified" approach, we might quickly find ourselves limited in more complex cases.

However, I think that by slightly modifying your proposal, we could achieve something interesting.

For example:

$builder->addRowAction('multipleActions', DropdownActionType::class, [
    'actions' => [
        'update' => ['type' => LinkActionType::class, 'options' => ['label' => 'Update something']],
        'delete' => ['type' => FormActionType::class, 'options' => ['label' => 'Delete something']],
    ]
]);

I’m not sure if I am entirely convinced by this proposal.

Advantages:

• No use of callbacks
• No need to inject a partial ActionBuilder, avoiding potential unexpected complexity from interface
• Ability to use custom ActionTypes

Disadvantages:

• Moves away from the preferred approach of using builders to instantiate
• Using arrays of arrays doesn’t seem relevant

Another proposal:

$builder->addRowAction('multipleActions', DropdownActionType::class, [
    'actions' => [
        new DropdownActionItem('update', LinkActionType::class, ['label' => 'Update something']),
        new DropdownActionItem('delete', FormActionType::class, ['label' => 'Delete something']),
    ]
]);

Advantages:

• No callbacks
• No partial ActionBuilder
• No arrays within
• Ability to use custom ActionTypes

Disadvantages:

• Doesn’t use the builder (I'm not sure if it's really a disadvantages)

It would be ideal if 'actions' could accept either an array or a callback (yeah I know, another callback !) to retrieve the values of the current row:

$builder->addRowAction('multipleActions', DropdownActionType::class, [
    'actions' => function (Product $product) {
        return [
            new DropdownActionItem('update', LinkActionType::class, [
                'label' => 'Update something', 
                'url' => $this->router->generate('app_update_product', ['id' => $product->getId()])
            ]),
            new DropdownActionItem('delete', FormActionType::class, ['label' => 'Delete something']),
        ];
    }
]);

[Kreyu]

Clearly, this approach seems much simpler. On the other hand, I also find that we lose the reuse of other actions.

Okay, let's ignore the options and builder API for a moment. Reusing other action types seems cool, and I would be all-in on that idea, but how do we render them?

If I use a dropdown action, I would expect them to render as a classic dropdown list:

If we want to reuse other actions, this is impossible to render in that way, because each action type HTML may or may not be suitable for a dropdown (take into account custom action types and themes):

We cannot modify each child action HTML on render to make it suitable for a dropdown, as each type and theme can render those differently. Because we are not limiting how each action type can be configured and rendered (total freedom!), this also prevents us from reusing them in a dropdown without creating a monstrosity of a HTML 😬

In fact, I would say, that we:

• can create a dropdown action, that contains a list of "links" - which is still powerful, even with simple HTML attributes you can attach a JavaScript handler, open a modal, or much much more;
• cannot create groups of ActionInterface, because the freedom of their definition prevents us from rendering them together;

[alexandre-castelain]

Maybe a naive question, but: wouldn't it be possible to create a specific "sub" block?

For the ActionLinkType, for example:

• First, we look for a block dropdown_action_link_value
• If it is not present, we look for an action_link_value

This way, depending on the theme, we can "redesign" the way the HTML is displayed in the Dropdown without touching the default HTML.

I am not used to working with Twig themes like this, so I might be completely off track!

[Kreyu]

Maybe a naive question, but: wouldn't it be possible to create a specific "sub" block?

Yeah, we could do it in the same way the column is split into two parts - a "header" and a "value".

Currently, action Twig blocks are suffixed with _value, which I'm not fond of. Maybe we should seize the opportunity and refactor it into _control, and for dropdowns, something like _dropdown_control. Then, add Twig function named data_table_dropdown_action, similar to data_table_action, that will render the proper block (with fallback to parent type block).

Also, we would have to add something like buildDropdownView() method to the ActionTypeInterface, that lets you build a separate ActionView for the dropdown. This, in my opinion, will be inconvenient, because most action types will require duplicate logic inside of buildView() and buildDropdownView(), with resolving the options and passing them to the view. Then, passing options specific for a specific view will be hard. For example, if we pass HTML attributes using the attr option, do they belong to the "regular" action view, or the dropdown one? Wouldn't that require adding separate option prefixed with dropdown_? It seems like it doubles the complexity of an action configuration 😞. Maybe this is fine, but I wonder if this is not too complicated for a simple dropdown.

I know it would be possible, but I really don't want to increase complexity like that just for the sake of a single dropdown action type.

On the other hand... shouldn't this be a theme-specific feature? For example, we could add a theme that groups actions into one dropdown, and renders them properly (the theme will simply contain proper HTML for each action type). This way, we can use this theme in data tables that require this feature. This also means, that we would no longer mix and match inline actions with dropdown actions, but I think that's how its done in, for example, a EasyAdmin - actions are either inline or not. For now, we could do something like bootstrap_5_dropdown_actions.html.twig, that, when imported after the Bootstrap 5 theme, it would render actions as a dropdown, instead of a default inline method. That method would be really simple (just a one twig file), totally optional, and, I think, a good compromise between complexity and functionality.

[alexandre-castelain]

Hi Kreyu!

Sorry for my very slow response, I've been quite busy these past few months. I'll make some time to work on this bundle until the end of the year. I hope to contribute to the project so it can meet some of my needs.

If we make enough progress, I might be able to allocate some work time to it as well.

After rereading our conversation several times, I think I initially proposed something too complex. The idea is simply to be able to add multiple actions in a dropdown.

I would like it to be relatively simple to add a new type of action to a dropdown, without too many files needing modification. I like the idea of simply integrating a twig file to switch from inline to dropdown.

Now, it would be interesting to define how to create these actions, which "builder" to use. And also, where do we use it? In the "DataTableType"? In the "DropdownActionType"? Somewhere else?

I really want to work on this point, but I don't know exactly how to start. Do you have any advice? If you'd prefer to discuss it verbally, we can chat elsewhere, whichever you prefer.

Have a great day!

[Kreyu]

Hey @alexandre-castelain! I've tried doing something related to dropdown actions in one of my projects and now I have some conclusions. I'll collect my thoughts and try to write it down this week.

Cheers!

[Kreyu]

Sorry for the delay.

In one of my projects, I had a data table with 6+ actions (icons only), which started to be... not quite intuitive. Previously, I suggested:

On the other hand... shouldn't this be a theme-specific feature? For example, we could add a theme that groups actions into one dropdown, and renders them properly (the theme will simply contain proper HTML for each action type). This way, we can use this theme in data tables that require this feature.

In custom theme, I had changed the column_actions_value to render actions in a dropdown:

{% block column_actions_value %}
    <div class="text-end" data-turbo="false">
        <div class="dropdown position-static">
            <button class="btn dropdown-toggle align-text-top"
                    data-bs-toggle="dropdown"
                    data-bs-boundary="window"
                    aria-expanded="false"
            >
                {{ 'data_table.actions'|trans }}
            </button>
  
            <div class="dropdown-menu dropdown-menu-end">
                {% for action in actions %}
                    {# adding "dropdown" variable to the context #}
                    {{ data_table_action(action, { dropdown: true }) }}
                {% endfor %}
            </div>
        </div>
    </div>
{% endblock %}

Then, I had changed the action_button_value, action_form_value blocks to render actions as dropdown items if dropdown variable was defined and equal to true, e.g.:

{% block action_button_value %}
    {% if dropdown ?? false %}
        {% set attr = attr|merge({ class: (attr.class|default('') ~ ' dropdown-item mx-1')|trim }) %}
        {% set icon_attr = icon_attr|merge({ class: (icon_attr.class|default('') ~ ' me-1')|trim }) %}

        {# ... #}
    {% endif %}

    {{ parent() }}
{% endblock %}

And that worked fine until I tried clicking the confirmable action. The modal doesn't work that way, because it is rendered inside the dropdown. So inside the column_actions_value I had to iterate through actions again and render the confirmation modals outside the dropdown, and disable rendering it inside the action block.

{% block column_actions_value %}
    {# ... render dropdown and its items #}

    {# Render confirmation and modal action type modals outside of dropdown, otherwise it won't render #}
    {% for action in actions %}
        {% if action.vars.confirmation %}
            {% with action.vars %}
                {{ block('action_confirmation_modal') }}
            {% endwith %}
        {% endif %}
    {% endfor %}
{% endblock %}

{% block action_button_value %}
    {# ... configure attributes to render the action as dropdown item #}

    {# Set confirmation to false, as we're manually rendering confirmation modal outside of the dropdown #}
    {% set confirmation = false %}

    {{ parent() }}
{% endblock %}

This is... fine, but shows a really noticable issue - if you render an action as dropdown item, you cannot provide additional HTML to render outside the dropdown. For example, if you create ModalActionType, in this scenario, you cannot render the modal inside the action_modal_value block, because it would be rendered inside the dropdown. It would require additional template block that would render outside the dropdown, and I think this complicates things more than it helps...

So, doing it this way:

• doesn't require additional configuration, you just use the theme, and ALL row actions are rendered inside a single dropdown
• you cannot render some actions as buttons and some inside a dropdown, since the theme renders ALL row actions as a dropdown
• requires doing weird stuff in Twig that will be hard to understand and maintain in applications 😅

I'm open for making this as simple as possible, for example a single DropdownActionType that renders as a dropdown with array of sub-actions, e.g. DropdownItemActionType. Unfortunately, I think that this will have the same issues with rendering stuff outside of the dropdown (e.g. modals), but maybe I am overthinking it waaay to far.

[alexandre-castelain]

No worries, we all have a life! :)

I find the thought process interesting. A few remarks:

• No possibility to add HTML outside of the action, for example, to open a modal or a confirmation message.
  In our project, we solved this issue using a Stimulus controller to decouple things. In this bundle, I think it should be possible to choose a Stimulus controller that handles modals and confirmation. These could be bundle-specific controllers or custom ones, it doesn’t matter as long as the correct events are triggered.
• All actions of a row are rendered in a single dropdown.
  For me, this approach doesn’t work. It’s not practical for many use cases. Example: a list of "secondary" actions and a primary action (e.g., validating an order) or deleting the row.

In the end, perhaps splitting the views for greater simplicity is the way to go: one "inline" view and one "list" view. That’s what we use in our project, and it turns out to be quite simple to manage—the code remains clear and easy to maintain.

[Kreyu]

No possibility to add HTML outside of the action, for example, to open a modal or a confirmation message.
In our project, we solved this issue using a Stimulus controller to decouple things. In this bundle, I think it should be possible to choose a Stimulus controller that handles modals and confirmation. These could be bundle-specific controllers or custom ones, it doesn’t matter as long as the correct events are triggered.

I'm not sure whether this bundle should provide its own scripts for handling the modals, especially if Bootstrap provides its own scripts used in most cases (I think, speaking from experience).

For me, what seems to be most intuitive, is simply a DropdownActionType that will render an action as a dropdown:

$builder->addRowAction('advanced', DropdownActionType::class);

Now, the dropdown requires items, so we provide them via items option:

$builder->addRowAction('advanced', DropdownActionType::class, [
    'items' => [
        $builder->createRowAction('toggle', DropdownItemActionType::class),
        $builder->createRowAction('remove', DropdownItemActionType::class),
    ],
]);

Thanks to that, we can mix and match various action types:

$builder
    ->addRowAction('details', ButtonActionType::class)
    ->addRowAction('update', ButtonActionType::class)
    ->addRowAction('advanced', DropdownActionType::class, [
        'items' => [
            $builder->createRowAction('toggle', DropdownItemActionType::class),
            $builder->createRowAction('remove', DropdownItemActionType::class),
        ],
    ])
;

Rendering those action types seems simple:

{# @KreyuDataTable/themes/bootstrap_5.html.twig #}

{% block action_dropdown_value %}
    <div class="dropdown">
        <button class="btn btn-primary" data-bs-toggle="button">...</button>
        
        <ul class="dropdown-menu">
            {% for item in items %}
                <li>{{ data_table_action(item) }}</li>
            {% endfor %}
        </ul>
    </div>
{% endblock %}

{% block action_dropdown_item_value %}
    <a class="dropdown-item">...</a>
{% endblock %}

Now, let's assume that we have a ModalActionType that renders a button that opens a modal. In most basic scenario, the modal is rendered next to the button:

{% block action_modal_value %}
    <button data-bs-toggle="modal" data-bs-target="#modal">...</button>
    <div id="modal">...</div>
{% endblock %}

If we use this action type as a dropdown item, the dropdown will render the button, what most likely will look weird and out of place. I think creating a separate ModalDropdownItemActionType is the way:

{% block action_modal_dropdown_item_value %}
    <a class="dropdown-item" data-bs-toggle="modal" data-bs-target="#modal">...</a>
    <div id="modal">...</div>
{% endblock %}

As I described before, because the modal is rendered inside the dropdown, it won't show up. It has to be rendered outside the modal. I think this shows that some action types should allow rendering via multiple blocks:

{% block action_dropdown_value %}
    <div class="dropdown">
        <button class="btn btn-primary" data-bs-toggle="button">...</button>
        
        <ul class="dropdown-menu">
            {% for item in items %}
                {# This will render "action_*_value" #}
                <li>{{ data_table_action(item, 'value') }}</li>
            {% endfor %}
        </ul>
        
        {# This will render "action_*_outer" #}
        {{ data_table_action(item, 'outer') }}
    </div>
{% endblock %}

{% block action_modal_dropdown_item_value %}
    <a class="dropdown-item" data-bs-toggle="modal" data-bs-target="#modal">...</a>
{% endblock %}

{% block action_modal_dropdown_item_outer %} 
    <div id="modal">...</div>
{% endblock %}

This would require extending the Twig functions to accept a block suffix instead of always rendering the value. Not sure whether this "rendering system" would be intuitive enough.

Speaking of value, I think changing it to control will be more intuitive in the context of actions.

[alexandre-castelain]

As I described before, because the modal is rendered inside the dropdown, it won't show up. It has to be rendered outside the modal. I think this shows that some action types should allow rendering via multiple blocks:

I didn't explain it earlier, but in my mind, the content of the modal is fulfilled with an AJAX request. So the ModalDropdownItemActionType should just contain the URL (or route + route params) where it can find the content as raw HTML.

If we don't do this way, for each modal we should create a specific form theme. I don't find this really fancy.

$builder
    ->addRowAction('details', ButtonActionType::class)
    ->addRowAction('update', ButtonActionType::class)
    ->addRowAction('advanced', DropdownActionType::class, [
        'items' => [
            $builder->createRowAction('toggle', DropdownItemActionType::class),
            $builder->createRowAction('remove', DropdownItemActionType::class),
        ],
    ])
;

I'll work on that and show you what I manage to do.

[alexandre-castelain]

Hi @Kreyu !

With the MR I created, it's possible to have a dropdown, with multiple actions, in the table.
It's possible to have multiple DropdownAction in the same table, combine them with "classic" actions.

            ->addRowAction('actionColumn', DropdownActionType::class, [
                'actions' => [
                    $builder->createRowAction('update', LinkDropdownItemActionType::class, [
                        'href' => function (Product $product) {
                            return $this->urlGenerator->generate('app_product_update', [
                                'id' => $product->getId(),
                            ]);
                        }
                    ])
                ]
            ])

In the end, I just created new ActionType, and it's really easy to add new ones based on other actions : just extend them (via the getParent method). This way, you benefit from the logic already present for the basic action, but can add specific behaviour and for the template, it's also easy, because you have the cascading block flow.

For the LinkDropdownItemActionType, you have the action_link_dropdown_item_control > action_link_control > action_control. I kind of like it.

What do you think about this ? I will wait the changes on the FormActionType and ButtonActionType to go further on this.

[Kreyu]

It's possible to have multiple DropdownAction in the same table, combine them with "classic" actions.
...
In the end, I just created new ActionType, and it's really easy to add new ones based on other actions : just extend them (via the getParent method). This way, you benefit from the logic already present for the basic action, but can add specific behaviour and for the template, it's also easy, because you have the cascading block flow.

Great! That's what I had in mind as well. I will review the code and add comments there, thanks!

[alexandre-castelain]

I'm not sure we should merge it yet, there's no documentation and no test. I should give it a try !

[Kreyu]

Do we need this controller at all? I don't think we really need a custom solution to dropdowns - the Bootstrap provides its own scripts we're using with data params, and base theme doesn't need a functional dropdown, because it is not meant to be used directly. I would assume that themes that extend the base theme should provide their own implementation of dropdown, like we do with Bootstrap.

[alexandre-castelain]

I don't know why, but I didn't manage to make any dropdown from bootstrap properly.

If this works well with bootstrap, I totally agree with you, we can remove this controller.

[Kreyu]

I'll have to check the issue with Bootstrap modals in an application and try some things. I'll come back to you.

[Kreyu]

I'm not sure we should merge it yet, there's no documentation and no test. I should give it a try !

I think it would be inappropriate to require tests when there's no tests for actions at all 😄. Recently, I've added tests for all column types, and I will do it soon for actions, so no worries - I can add test for dropdown-related types later.

Adding documentation would be excellent, even simple reference pages for new action types. I can do it myself later if you prefer, or if you encounter some issues with documentation. Running the docs locally should be easy though: https://data-table-bundle.swroblewski.pl/docs/contributing.html#documentation

Overall, great work, thank you! I've added some comments to the review.

[Kreyu]

Now that I think about it, what would you expect with ActionTypeExtension that extends a LinkActionType? Should it automatically extend the LinkDropdownItemActionType as well?

Because currently it will, because LinkDropdownItemActionType extends LinkActionType, and it is not possible to prevent that. Maybe its fine, maybe it's not. That's the way Symfony Form type extensions work and that's why it's like that here.

Recently I've been thinking whether we should ditch the inheritance in core completely to prevent issues with extensions. The same way Symfony Form NumberType does not extend TextType, we should probably not extend TextColumnType in NumberColumnType. That is... slightly complicated case and depends on a situation. This doesn't block this pull request though, I'm just thinking out loud 😁

[Kreyu]

Thanks for the changes after the review, I'll try to test it out as soon as possible in a real application!

[Kreyu]

Oops, sorry, ignore that review request.

[Kreyu]

Thanks again for all your efforts @alexandre-castelain. I've added small improvements here and there and added the documentation. Unfortunately, I had to (or I think I had to, I don't really know) create a separate branch for that, since I was unable to push to your branch. Merged in #170.

I'll draft a new release next week, since there are multiple changes waiting to be released.