- Issue created by @f.mazeikis
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
I wrote this issue summary together with @f.mazeikis ๐ค
- Merge request !1206Add default file dependency handling during exports and imports โ (Open) created by Unnamed author
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
@mstrelan pointed out we have the 8.4 polyfill in 11.2 but not in 11.1
- ๐ฌ๐งUnited Kingdom f.mazeikis Brighton
Pushed all the work so far, including working tests. @larowlan had some feedback, I think I've addressed most of it, but happy to get back to it if there's more.
Moving into ready for review.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
#6: That should be fine, because XB bumped its requirement to 11.2 in ๐ PHPUnit Next Major tests failing Active ? That's what's in 0.5.0-alpha1 โ , so <0.5 is for Drupal <11.2, >=0.5 is for Drupal >=11.2
That was pretty fast! Reviewingโฆ ๐ค
- ๐ซ๐ฎFinland lauriii Finland
We can work around this in beta by uploading images to media library and manually referencing them in component code. This is obviously not great and doesn't allow components to be moved from environment to another so we definitely still need this. Just trying to avoid rushing this before the beta.
- ๐ฆ๐บAustralia mstrelan
Re polyfill:
Regardless of core version no reason XB couldn't add it as a dep, just give it a loose constraint so it doesn't conflict with core. - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Looking promising!
- This needs to have preliminary internal HTTP API support. "preliminary", because it might be imperfect, and that's fine. But we need to have the FE folks unblocked to build the entire feature; we can then still refine it later ๐
- That MUST come with updated test coverage in
\Drupal\Tests\experience_builder\Functional\XbConfigEntityHttpApiTest::testJavaScriptComponent()
- Some missing test coverage to ensure that saved config entities in config entity storage do not actually contain these base64 blobs
- ๐ฌ๐งUnited Kingdom f.mazeikis Brighton
Addressed everything on MR and in #11.
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Great work @f.mazeikis - did another review and added some comments/observations
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ Responded to @f.mazeikis' most pressing question; didn't do a full review, but โฆ the scope here is now no longer so narrow, but generic, thanks to โจ Define JSON Schema $refs for linking/embedding videos and linking documents Active + โจ Add a Video prop type to the Code Component editor Active .
- ๐ฌ๐งUnited Kingdom f.mazeikis Brighton
I've addressed most of the feedback, replied with reasoning for the rest. This might not be complete, but a second pass review and further feedback on Comment on lines +802 to +804 would be beneficial.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
I'll review tomorrow โ it's possible @larowlan will beat me to it, so not yet self-assigning.