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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c056827
#2167: Keep in mind order of multiselect.
haringsrob Mar 30, 2023
831cd2d
cleanup.
haringsrob Mar 30, 2023
3c47995
Merge branch '3.x' into 2167-port
ifox Apr 12, 2023
7b6b05e
Merge branch '3.x' into 2167-port
ifox Feb 4, 2024
ca9df8f
Remove node-forge and update dependencies
ifox Feb 4, 2024
0dfe6d8
Update default compiled assets
ifox Feb 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion frontend/js/components/VSelect.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,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.

ping @antonioribeiro

Copy link
Member

@antonioribeiro antonioribeiro Jan 26, 2024 •
edited
Loading

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()
            )
        )
);

this.value = []
for (const v in value) {
const matches = this.options.filter(o => {
return o.value === value[v]
})

if (matches[0]) {
this.value.push(matches[0])
}
}
}
} else {
this.value = this.options.find(o => {
Expand Down
670 changes: 340 additions & 330 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

43 changes: 21 additions & 22 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,23 @@
"docs": "rm -rf docs/_build/* && ./vendor/bin/testbench twill:staticdocs:serve"
},
"dependencies": {
"@alpinejs/mask": "^3.13.3",
"@tiptap/extension-hard-break": "^2.1.16",
"@tiptap/extension-horizontal-rule": "^2.1.16",
"@tiptap/extension-link": "^2.1.16",
"@tiptap/extension-placeholder": "^2.1.16",
"@tiptap/extension-table": "^2.1.16",
"@tiptap/extension-table-cell": "^2.1.16",
"@tiptap/extension-table-header": "^2.1.16",
"@tiptap/extension-table-row": "^2.1.16",
"@tiptap/extension-text-align": "^2.1.16",
"@tiptap/extension-underline": "^2.1.16",
"@tiptap/pm": "^2.1.16",
"@tiptap/starter-kit": "^2.1.16",
"@tiptap/vue-2": "^2.1.16",
"alpinejs": "^3.13.3",
"@alpinejs/mask": "^3.13.5",
"@tiptap/extension-hard-break": "^2.2.1",
"@tiptap/extension-horizontal-rule": "^2.2.1",
"@tiptap/extension-link": "^2.2.1",
"@tiptap/extension-placeholder": "^2.2.1",
"@tiptap/extension-table": "^2.2.1",
"@tiptap/extension-table-cell": "^2.2.1",
"@tiptap/extension-table-header": "^2.2.1",
"@tiptap/extension-table-row": "^2.2.1",
"@tiptap/extension-text-align": "^2.2.1",
"@tiptap/extension-underline": "^2.2.1",
"@tiptap/pm": "^2.2.1",
"@tiptap/starter-kit": "^2.2.1",
"@tiptap/vue-2": "^2.2.1",
"alpinejs": "^3.13.5",
"axios": "^0.27.2",
"core-js": "^3.35.0",
"core-js": "^3.35.1",
"cropperjs": "^1.6.1",
"date-fns": "1.30.1",
"fine-uploader": "^5.16.2",
Expand All @@ -41,16 +41,16 @@
"smartcrop": "^2.0.5",
"tinycolor2": "1.6.0",
"truncate-utf8-bytes": "1.0.2",
"vue": "^2.7.14",
"vue": "^2.7.16",
"vue-select": "^3.20.2",
"vue-timeago": "^5.1.3",
"vuedraggable": "2.24.3",
"vuetrend": "^0.3.4",
"vuex": "^3.6.2"
},
"devDependencies": {
"@area17/a17-tailwind-plugins": "^3.10.0",
"@babel/eslint-parser": "^7.23.3",
"@area17/a17-tailwind-plugins": "^3.12.0",
"@babel/eslint-parser": "^7.23.10",
"@tailwindcss/forms": "^0.5.7",
"@tailwindcss/typography": "^0.5.10",
"@vue/cli-plugin-babel": "~5.0.8",
Expand All @@ -64,10 +64,9 @@
"eslint-plugin-node": "11.1.0",
"eslint-plugin-promise": "4.3.1",
"eslint-plugin-standard": "4.1.0",
"eslint-plugin-vue": "^9.20.1",
"node-forge": "^1.3.1",
"eslint-plugin-vue": "^9.21.1",
"prettier": "1.19.1",
"sass": "^1.69.7",
"sass": "^1.70.0",
"sass-loader": "^8.0.2",
"svg-spritemap-webpack-plugin": "^4.5.0",
"tailwindcss": "^3.4.1",
Expand Down
41 changes: 40 additions & 1 deletion src/Repositories/ModuleRepository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,46 @@

public function updateMultiSelect(TwillModelContract $object, array $fields, string $relationship): void
{
$object->$relationship()->sync($fields[$relationship] ?? []);
// get the current list of items
$items = $fields[$relationship];

Check warning on line 637 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L637 

Added line #L637 was not covered by tests

if (blank($items)) {

Check warning on line 639 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L639 

Added line #L639 was not covered by tests
// let's just delete all the items
$object->$relationship()->sync([]);

Check warning on line 641 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L641 

Added line #L641 was not covered by tests

return;

Check warning on line 643 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L643 

Added line #L643 was not covered by tests
}

$newItems = [];

Check warning on line 646 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L646 

Added line #L646 was not covered by tests

$position = 1;

Check warning on line 648 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L648 

Added line #L648 was not covered by tests

foreach ($items as $key => $item) {
if (is_numeric($item)) {

Check warning on line 651 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L650-L651 

Added lines #L650 - L651 were not covered by tests
// if it's not an array of pivot fields already, set the position
$newItems[$item] = ['position' => $position];

Check warning on line 653 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L653 

Added line #L653 was not covered by tests
} else {
if (is_array($item)) {

Check warning on line 655 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L655 

Added line #L655 was not covered by tests
// if it's an array of pivot fields and no position is set, set the position
if (!isset($item['position'])) {
$item['position'] = $position;

Check warning on line 658 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L657-L658 

Added lines #L657 - L658 were not covered by tests
}
}

// go with whatever we have from the request
$newItems[$key] = $item;

Check warning on line 663 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L663 

Added line #L663 was not covered by tests
}

$position++;

Check warning on line 666 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L666 

Added line #L666 was not covered by tests
}

try {
$object->$relationship()->sync($newItems);
} catch (\Throwable $e) {

Check warning on line 671 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L670-L671 

Added lines #L670 - L671 were not covered by tests
// If an error occurs, maybe because the pivot table doesn't have the 'position' column,
// we will just keep the old implementation
$object->$relationship()->sync($fields[$relationship]);

Check warning on line 674 in src/Repositories/ModuleRepository.php

View check run for this annotation

Codecov / codecov/patch

src/Repositories/ModuleRepository.php#L674 

Added line #L674 was not covered by tests
}
}

public function addRelationFilterScope(
Expand Down
1 change: 1 addition & 0 deletions twill-assets/assets/twill/js/chunk-common.30b7c4b6.js
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion twill-assets/assets/twill/js/chunk-common.57415794.js
View file
Open in desktop

This file was deleted.

37 changes: 0 additions & 37 deletions twill-assets/assets/twill/js/chunk-vendors.5151f711.js
View file
Open in desktop

This file was deleted.

37 changes: 37 additions & 0 deletions twill-assets/assets/twill/js/chunk-vendors.a68a3d70.js
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions twill-assets/assets/twill/js/main-buckets.4f719092.js
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion twill-assets/assets/twill/js/main-buckets.8033e5ad.js
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion twill-assets/assets/twill/js/main-dashboard.59f736b4.js
View file
Open in desktop

This file was deleted.

Loading
Loading