Problem
Libraries API module is supported by Paragraphs to optionally load "Sortable" library.
Libraries API has changed the approach used in Drupal 8, so that a number of functions are deprecated there including the one used by Paragraphs. However neither the documentation page nor the code of Libraries API explains the up-to-date way to detect libraries.
We should avoid using deprecated functions to make Paragraphs D9-ready.
Proposed solutions
Figure out the up-to-date way to use Libraries API and implement it in Paragraphs.
OR
Remove Libraries API support and use the fallback option already implemented in Paragraphs. (It might make sense since Libraries API looks abandoned for Drupal 8.)
Comment | File | Size | Author |
---|---|---|---|
#19 | 3055618-19.patch | 31.23 KB | sidharthap |
| |||
#19 | interdiff-17-19.txt | 888 bytes | sidharthap |
#17 | 3055618-17.patch | 30.36 KB | sidharthap |
#17 | interdiff-15-17.txt | 16.94 KB | sidharthap |
#15 | 3055618-15.patch | 45.27 KB | sidharthap |
Comments
Comment #2
BerdirSee #3090457: Use sortable library from core, combined with requiring 8.8, we can simply remove that now.
Comment #3
BerdirComment #4
sidharthapThis patch removes own sortablejs library. tested the Drag & drop functionality with core sortable library.
Comment #5
pstewart CreditAttribution: pstewart commentedIn backporting 3055618-4.patch to apply it to 1.10 I ran into a problem with
hook_library_info_alter
not behaving as expected. Looking at the code block removed, the function returns libraries unchanged for D8.8, so the entire alter hook can actually be removed given that this patch enforces D8.8. This worked for me when patching 1.10Comment #6
BerdirThat is correct.
That said, it might be easier to use the path from #3090457: Use sortable library from core, which will dynamically use the library from core already.
Comment #7
sidharthap@pstewart , @Berdir Thank you. I tested the drag and drop functionality on a new setup after removing alter hook and its working.
Comment #8
BerdirYes, that looks good so far, now we also need to take care of the remaining 8.8 deprecations like entity_get(_form)_display().
Comment #9
sidharthapUpdated patch with https://www.drupal.org/project/paragraphs/issues/3055618#comment-13428312 comments.
Comment #10
sidharthapsmall correction.
Comment #11
Berdir@sidharthap: Thanks, please always provide an interdiff when making changes to an existing patch, so that it is easy to see what changed: https://www.drupal.org/documentation/git/interdiff
Comment #12
BerdirStill a few things left to do:
You need to add match_limit: 10 to core.entity_form_display.paragraph.from_library.default.yml below match_operator. And same for some other files.
For these, you need to find the matching assertTrue/False that are not actually checking for a boolean value but a string or so and change it accordingly.
See message, I think this is just expectedDeprecation() now.
Comment #13
sidharthapThank you @Berdir.
Updated patch without setExpectedException().
Comment #14
Berdirisset() returns true or false, so that was correct. You need to be careful to only change the lines that were actually not a boolean.
same here and many others. This could use assertCount() or just don't change it.
Comment #15
sidharthapThank you @Berdir. Updated patch
Comment #16
BerdirStill 3 test fails and most calls are not correctly changed.
Only checked the first 3 examples, but you can see that they all return true/false only, so assertTrue/False is the correct thing to do here and they should not be changed at all.
Comment #17
sidharthapYes @Berdir, assertTrue/False should not be changed in case bool. reverted such cases.
Comment #18
BerdirWe're getting close here, did run the test on D9 locally and while a few still fail due to dependencies, most passed, only non-dependency related error was this:
1) Drupal\Tests\paragraphs\Unit\migrate\FieldCollectionsFieldInstanceSettingsTest::testFieldCollectionBadBundle
Error: Call to undefined method Drupal\Tests\paragraphs\Unit\migrate\FieldCollectionsFieldInstanceSettingsTest::setExpectedException()
Comment #19
sidharthapThank @Berdir.
Patch to remove setExpectedException.
Comment #20
BerdirAnd committed! Paragraphs 8.x-1.x-dev now requires Drupal 8.8 and should be fully compatible with Drupal 9.
Comment #23
jonathanshawLooks like you forgot to bump core requirement in composer.json: #3123420: Bump core requirement in composer.json