Skip to content
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

[3.x] Set position on multi selects #2204

Merged
merged 6 commits into from
Feb 4, 2024
Merged

[3.x] Set position on multi selects #2204

merged 6 commits into from
Feb 4, 2024

Conversation

haringsrob
Copy link
Contributor

Port of #2167

@@ -168,7 +168,16 @@
if (this.taggable) {
this.value = value
} else {
this.value = this.options.filter(o => value.includes(o.value))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ifox @antonioribeiro

I had to add this chunk of js to ensure that the options are set to the order of how the id's come in. I believe this also needs to be in 2.x as otherwise it will take the order of all the available options (which is not sorted)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antonioribeiro can you share more about your use case for storing the position on multiselect? How are you using the field: which options do you use and how are the values loaded?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@antonioribeiro antonioribeiro Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ifox, use case is basically when we need to read the selected items in the order we are selecting them.

On the screenshot below I selected "Education" first, because I want Education to be shown as the main sector in my blog post page.

Screenshot 2024-01-26 at 15 16 29

When I save and reload the model, Education is no more the first item and the frontend will show "Housing" as the main sector:

Screenshot 2024-01-26 at 15 14 26

It doesn't really matter how I'm getting these records later because their positions are all null on the relationship table:

Screenshot 2024-01-26 at 15 26 25

There will be no way for me to read them in the right order.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how I'm creating the form field to test on Twill 3, where I get the same behaviour:

$form->add(
    MultiSelect::make()
        ->name("sectors")
        ->inline()
        ->options(
            Options::make(
                Sector::all()
                    ->map(
                        fn($sector) => Option::make(
                            $sector->id,
                            $sector->title
                        )
                    )
                    ->toArray()
            )
        )
);

@area17 area17 deleted a comment from codecov bot Mar 30, 2023
@ifox ifox changed the title #2167: Keep in mind order of multiselect. [3.x] #2167: Keep in mind order of multiselect. Apr 12, 2023
@ifox ifox changed the title [3.x] #2167: Keep in mind order of multiselect. [3.x] Set position on multi selects Apr 12, 2023
@ifox ifox merged commit 663ce4a into 3.x Feb 4, 2024
9 of 12 checks passed
@ifox ifox deleted the 2167-port branch February 4, 2024 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants