LinkColumnType : why default target="_self"

[bdecarne]

Hi !

Is there any reason to make target option required and "_self" by default in the LinkColumnType ?

I have the impression that this behavior causes Turbo Drive to malfunction. I'm not a specialist in this library, but I get the impression that links with target=_self are ignored by Turbo.

EDIT : i just saw this issue hotwired/turbo#1113

[bdecarne]

To complete my message :

I think the <turbo-frame> around the table should have the property target="_top" to force column or action links to navigate outside the frame. Then, pagination links should have the property data-turbo-frame="_self" to force them to render into the frame.

I can make the MR if it makes sense

[Kreyu]

Hey @bdecarne! Thanks for the report. I quickly glanced at one of my applications using the bundle, and I can confirm you're right.

Is there any reason to make target option required and "_self" by default in the LinkColumnType ?

To be honest, I simply looked at the docs of the anchor element, which described a _self is used by default. I thought that explicitly setting this attribute would change nothing. I'm up for making this option nullable by default.

So, as you suggested, this would require making following changes:

• adding target="_top" to the turbo-frame (changing base.html.twig template should be sufficient)
• adding data-turbo-frame="_self" to pagination controls (changing base.html.twig template should be sufficient)
• making target option nullable in LinkColumnType
  • we should also update the column_link_value block in base template to prevent adding empty target attribute
• making target option nullable in LinkActionType (which also makes it optional in ButtonActionType via type inheritance)
  • like in link column type, we should also update the action_link_value and action_button_value

I've noticed the action_button_value is exactly the same as action_link_value - saving this one for the small refactor of themes (#91).

Pull request would be highly appreciated, if you're willing to do that!

Cheers.

[Kreyu]

Upon further testing, I don't really understand what difference would the data-turbo-frame="_self" make. Is it really required? If so, I think it should also be applied on sortable column headers. But when I'm testing this on one of my apps, pagination button with and without this attribute work the same (or maybe I am missing something here)

[bdecarne]

You are right : sortable links should have this property too.

It works without, but the whole page is loaded instead of just the frame. It makes the <turbo-frame> tag useless, i guess.

[Kreyu]

Alright, thanks for confirming. I was focused solely on the fact that the request was sent asynchronously, not that it targeted the whole page, instead of a frame.

[Kreyu]

Thanks for the help! Included in the v0.18.1 release.

[davidromani]

Could you add a short example about how to make a table works inside a Turbo frame? I need to sort and paginate a table without full load page refresh (if it's possible)

[Kreyu]

Could you add a short example about how to make a table works inside a Turbo frame? I need to sort and paginate a table without full load page refresh (if it's possible)

Hey @davidromani, all built-in themes wrap each data table in their own turbo frame, so it works out-of-the-box:

data-table-bundle/src/Resources/views/themes/base.html.twig

Lines 12 to 24 in 2af2c8e

	<turbo-frame id="kreyu_data_table_{{ name }}" target="_top">
	<div
	data-controller="{{ stimulus_controllers|join(' ') }}"
	data-kreyu--data-table-bundle--state-url-query-parameters-value="{{ url_query_parameters|default({})|json_encode }}"
	>
	{{ block('action_bar') }}
	{{ block('table') }}
	{% if pagination_enabled %}
	{{ data_table_pagination(pagination) }}
	{% endif %}
	</div>
	</turbo-frame>

When using a completely custom theme with custom kreyu_data_table block, just wrap the whole data table in turbo frame with unique id.

[davidromani]

Thanks @Kreyu

I'm figured out that what I need to achieve, indeed, is the following situation:

I need to disable Turbo Drive (any link clicked forces a full page reload) globally but I want to enable Turbo Frames (any link clicked inside the frame, I want to replace only the inside’s frame with the async response result)… it’s possible to achieve this use case?

[Kreyu]

Thanks @Kreyu

I'm figured out that what I need to achieve, indeed, is the following situation:

I need to disable Turbo Drive (any link clicked forces a full page reload) globally but I want to enable Turbo Frames (any link clicked inside the frame, I want to replace only the inside’s frame with the async response result)… it’s possible to achieve this use case?

I usually have turbo drive enabled throughout the project, and if something (e.g. link, form) requires it to be disabled, I just add the data-turbo="false" attribute.

Never done that in the opposite way, but maybe wrapping the whole data table in element with data-turbo="true" will re-enable the turbo drive just for the data table? Take a look: https://turbo.hotwired.dev/handbook/drive#disabling-turbo-drive-on-specific-links-or-forms

To reenable when an ancestor has opted out, use data-turbo="true":

<div data-turbo="false">
  <a href="/" data-turbo="true">Enabled</a>
</div>

So, maybe something like this will work?

<div data-turbo="true">{{ data_table(products) }}</div>

{# or in theme #}

{% extends '@KreyuDataTable/themes/bootstrap_5.html.twig' %}

{% block kreyu_data_table %}
    <div data-turbo="true">{{ parent() }}</div>
{% endblock %}

[davidromani]

@Kreyu Sorry for bothering you again.

Overriding the block kreyu_data_table with the wrapped example in theme works well in Turbo scope, but it fails because then the table HTML node looses the class="table table-bordered table-stripped table-hover sonata-ba-list" attribute.

I'm extending from @KreyuDataTable/themes/base.html.twig'

{% block kreyu_data_table %}
    <div data-turbo="true">
        {{ parent() }}
    </div>
{% endblock %}

{% block table %}
    <div class="row">
        <div class="col-xs-12">
            <div class="box box-primary">
                {% if title %}
                    <div class="box-header">
                        <h6 class="box-title">
                            {% if translation_domain is not same as false %}
                                {{ title | trans(title_translation_parameters, translation_domain) }}
                            {% else %}
                                {{ title }}
                            {% endif %}
                        </h6>
                    </div>
                {% endif %}
                <div class="box-body table-responsive no-padding">
                    {% with { table_attr: { class: 'table table-bordered table-stripped table-hover sonata-ba-list' } | merge(table_attr | default({})) } %}
                        {{ parent() }}
                    {% endwith %}
                </div>
            </div>
        </div>
    </div>
{% endblock %}

[Kreyu]

Hey @davidromani, sorry for late response, I've created a separate issue #152 for that, I'll try to fix this today.