Skip to content

[LiveComponent] WIP: sorting for LiveColletionType #2688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 2.x
Choose a base branch
from

Conversation

haivala
Copy link

@haivala haivala commented Apr 10, 2025

Q A
New feature? yes
Docs? yes/no
License MIT

LiveCollectionType has now new option allow_sort that allows sorting
Also added same options than buttons add and delete has for button move_up and move_down

@carsonbot carsonbot added Feature New Feature LiveComponent Status: Needs Review Needs to be reviewed labels Apr 10, 2025
@haivala
Copy link
Author

haivala commented Apr 10, 2025

I would actually move the buttons (delete, move) before the the form row in the template (line 9) so that they are not with the add button as they control the current entry type so they should be first?

Example
2025-04-10_140149-area

@Pechynho
Copy link

Pechynho commented Apr 12, 2025

I don't think this is a good way how to order thinks, because this does not respect object references.

Let me give you an example.

Let's say I have entities these categories.

[
  {
    "id": 1,
    "name": "Books"
  },
  {
    "id": 2,
    "name": "Movies"
  }
]

Category form type

<?php

namespace App\Form\Type;

use App\Entity\Category;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;

final class CategoryType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder->add('name', TextType::class, [
            'label' => 'Name',
            'required' => true,
            'empty_data' => '',
        ]);
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'data_class' => Category::class,
        ]);
    }
}

Usage in Live Component in instantiateForm method

protected function instantiateForm(): FormInterface
{
    return $this->formFactory->createNamed(
        name: 'form',
        type: CollectionType::class,
        data: $this->categories,
        options: [
            'label' => false,
            'entry_type' => CategoryType::class,
            'allow_add' => true,
            'allow_delete' => true,
            'delete_empty' => false,
            'prototype' => false,
        ]
    );
}

Initial data in LiveComponent formValues property

[
  {
    "name": "Books"
  },
  {
    "name": "Movies"
  }
]

Then I perform swap via your proposed method. This will give me these formValues

[
  {
    "name": "Movies"
  },
  {
    "name": "Books"
  }
]

After calling submitForm method in Live Component this is state of my entities

[
  {
    "id": 1,
    "name": "Movies"
  },
  {
    "id": 2,
    "name": "Books"
  }
]

As you can see, it just "swapped" data which are exposed to the form. Let say that I had some products in Books category and now they are all in category called Movies!

IMHO this proposed swapping algorithm can be used only in very simple scenarios and DEVs need to be aware of its quirks and limitations.

For true swapping you need at least one extra property, which holds item order.

@smnandre
Copy link
Member

I tend to agree with @Pechynho here.

I'm also a bit worried swapping names / ids of form elements may have other side effects on the front, especially for Stimulus. Same for re-rendering

For true swapping you need at least one extra property, which holds item order.

Not even sure here, as if you did not have one on the component swapped, i'm not sure they will rerender

@smnandre
Copy link
Member

@haivala do you want to try another approach ?

@Pechynho what would you recon: add an hidden field on each item dynamically ?

@Pechynho
Copy link

@smnandre hidden field could work. But there is need to order child_form before rendering them in Twig.

Something like this:

            {% set child_forms = [] %}
            {% for child_form in form.myCollection %}
                {% set child_forms = child_forms|merge({form: child_form, order: child_form.vars.data.order}) %}
            {% endfor %}
            {% for child_form in child_forms|sort((a, b) => a.order <=> b.order)|column('form') %}
                {{ form_row(child_form) }}
            {% endfor %}

Ofc, the order property name would have to be configurable.

@Pechynho
Copy link

Pechynho commented Apr 14, 2025

Let's say that API of LiveCollectionType would look like this.

protected function instantiateForm(): FormInterface
{
    return $this->formFactory->createNamed(
        name: 'form',
        type: LiveCollectionType::class,
        data: $this->categories,
        options: [
            'label' => false,
            'entry_type' => CategoryType::class,
            'allow_add' => true,
            'allow_delete' => true,
            'delete_empty' => false,
            'prototype' => false,
            'order_property_name' => 'order'
       ]);
}

Are we able to add 'order' hidden field automatically to CategoryType ? I don't think that it's possible and it would have to be provided by DEVs.

Other approach is not to modify CategoryType at all and store orders value in some internal LiveProp? And then set this value to object via FormEvent?

@kbond
Copy link
Member

kbond commented Apr 15, 2025

I also think a "position" property is required. I've done this with doctrine-extensions (sortable) and raw js.

@smnandre
Copy link
Member

Yep, and it would be much easier with JS, because we could change the order visually while not changing the order in HTTP response (and prevent all side effects).

Not sure there is a real possibility here (and people with small arrays or so can always use custom data actions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature LiveComponent Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants