Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
Account created on 26 December 2006, over 17 years ago
#

Merge Requests

More

Recent comments

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

drush cr alone is insufficient, you need to reinstall (because default config changed). ๐Ÿ™

IOW: always doing the steps at the top of /CONTRIBUTING.md is your safest bet. ๐Ÿ“Œ Remove the default value for the XB field and move components to a separate module Active will simplify those steps ๐Ÿ‘

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

The distraction/off-topic "CI allowed to fail" one mentioned in #3 is now fixed: #3472316-11: CI: remove `phpunit.image` override thanks to upstream fix โ†’ .

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Is this more accurate, @jessebaker?

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Oops! See #3472579-3: Tests are sometimes failing โ†’ , this is actually introducing CI warnings, so it's a regression. This needs more attention.

I'm confused why it was reported as 100% green here on d.o now ๐Ÿ˜ฌ

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

The warning started after ๐Ÿ“Œ CI: remove `phpunit.image` override thanks to upstream fix Fixed was merged, very suspicious.

Ahhhh โ€ฆ it looks like https://git.drupalcode.org/project/experience_builder/-/pipelines/275317 failed, I merged that prematurely! Revertingโ€ฆ

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Verifying fundamentals

For the experience_builder:heading SDC, the generated Component config entity does contain:

    style:
      field_type: list_string
      field_storage_settings:
        allowed_values:
          -
            value: primary
            label: primary
          -
            value: secondary
            label: secondary
      field_instance_settings: {  }
      field_widget: options_select
      default_value:
        value: primary

IOW: primary is selected as the default automatically, because that's the first example. So why is this not being respected? ๐Ÿค”

Turns out it is:

So the default aspect mentioned in #12 is an irrelevant distraction.

So: server allows selecting "nothing", but how to make client know this?

All that matters is what @lauriiii was getting at in #10: if an SDC marks a prop as optional, we should be able to detect that (the "no value entered" scenario). So how do we do that?

Turns out that form_select_options() hardcodes a particular special value: _none โ€” specifically for <select>. (Even the "list" field type's widget base class must hardcode this knowledge, see \Drupal\Core\Field\Plugin\Field\FieldWidget\OptionsWidgetBase::validateElement()). It can optionally be overridden using #empty_value.

Conclusion

Select.tsx, introduced in ๐Ÿ› Prop select lists don't affect the component Fixed , needs to be updated to be aware of this special value. If this special value is selected, then:

  1. the UI should interpret this as "no input specified by user"
  2. the UI should then delete this prop from the model, rather than setting "_none" as the value โ€” because that's the whole point of โœจ Contextual form values need to be integrated with Redux Active : to allow real-time updates of the preview based on information on the client side only
  3. that will then prevent this invalid model to be sent from the client to the server:
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

That was simpler than expected! ๐Ÿ˜„

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ“Œ Fix the visually broken "image" component instance: use FileUriItem's computed `url` property, not the stored `value` property Active was just fixed.

The two other blockers are in progress too: ๐Ÿ› Redux support for ImageWidget: `[image] String value found, but an object is required` Needs work is being worked on by @bnjmnm, ๐Ÿ› Adding a component with slots does not register the slots as children Fixed is being worked on by @balintbrews & @jessebaker.

Issue summary updated for you, @kristen pol ๐Ÿ‘

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Docs updated. Nits addressed. GIF added to issue summary.

Let's ship this! ๐Ÿš€

Thanks, @tedbow!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

#7: that's ๐Ÿ› Redux support for ImageWidget: `[image] String value found, but an object is required` Needs work , not this issue โ€” because you tested using ImageWidget, not MediaLibraryWidget.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

This blocks ๐Ÿ“Œ [PP-2] Add ComponentAuditabilityTest Active .

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ“Œ Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria Fixed landed weeks ago.

I think that now it makes sense to wait for ๐Ÿ“Œ [PP-2] Add ComponentAuditabilityTest Active , which will introduce part of what this aims to address: the beginnings of a UI that allows auditing what components are available in XB or not, and if not: why not.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ“Œ Mitigate class pollution from injected CSS Needs review is in.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Let's wait for @bnjmnm to reach a conclusion in ๐ŸŒฑ [META] Redux sync on ALL prop types, not just ones with a single [value] property Active first.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

So that is why that stupid error re-appeared! ๐Ÿคช

I knew we fixed it in ๐Ÿ“Œ Tweak PHPStan rules Fixed , so I did not understand why it re-appeared โ€” I overlooked that path! ๐Ÿ™ˆ

Thank you!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

I do totally believe in you! ๐Ÿ˜„

Also, I very much messed up the sample commits and issue summary ๐Ÿ™ˆ

Issue summary fixed, and mistake rectified on the MR: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2....

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

+1 โ€” I just didn't know you wanted to be involved like that โ€” definitely not back in May! ๐Ÿ˜…

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Another issue that is closely related to this: ๐Ÿ› Redux support for ImageWidget: `[image] String value found, but an object is required` Needs work .

I think the solution that ๐Ÿ“Œ Media Library integration (includes introducing a new main content renderer/`_wrapper_format`) Needs work introduced with data-media-file is not scalable/sustainable. But I do think there also is an opportunity to solve this generically.

See https://git.drupalcode.org/issue/experience_builder-3463842/-/tree/34638... for an outline (explained in 3 commits) of what I have in mind ๐Ÿค“ (Although that surely won't work for real-time updates, only for AJAX updates, which is the case for complex widgets like image, media library, etc.)

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Note: AFAICT this will need to call ImageItem::preSave() for it to work, which is called by massageFormValuesTemporaryRemoveThisExclamationExclamationExclamation(). But that is only called upon actually saving an entity, and ever since ๐Ÿ› Unable to save node article form โ€” remove obsolete TwoTerribleTextAreasWidget + fix duplicate `XB:image` SDC Fixed , that's not called at all anymore.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

I figured out the root cause: this happens when uploading a new image using the ImageWidget.

There simply is no Redux support yet for ImageWidget. That's what ๐ŸŒฑ [META] Redux sync on ALL prop types, not just ones with a single [value] property Active is for.

As of yesterday, ๐Ÿ“Œ Media Library integration (includes introducing a new main content renderer/`_wrapper_format`) Needs work landed and introduced hardcoded special support for MediaLibraryWidget, with experience_builder_preprocess_media_library_item__widget() generating a data-media-file attribute containing the updated field properties.

Phased solution:

  1. IMMEDIATE WORK-AROUND: Just install the Media Library module and use the Media Library Widget ๐Ÿ‘
  2. MR here: introduce similar behavior for ImageWidget, and piggy-back on what #3454173 introduced for Media.
  3. Long-term: generic solution in ๐ŸŒฑ [META] Redux sync on ALL prop types, not just ones with a single [value] property Active .

MR incoming.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Tests aren't passing.

๐Ÿ™ Please don't mark an issue as needing review until the MR passes tests!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

#8:

  • Possibly. Let's wait for there to be 3 occurrences, to avoid premature abstraction. For now, please add an @see to the other place.
  • Yes ๐Ÿ™
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

So close! ๐Ÿคฉ

Great work here ๐Ÿ˜Š

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Figured out the root cause while pairing with @tedbow!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

wim leers โ†’ created an issue.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

AFAICT tests are present and passing now ๐Ÿ‘

@jessebaker has pointed out an oversight over at https://git.drupalcode.org/project/experience_builder/-/merge_requests/2..., so marking .

In there, he also points out a scenario that is currently broken and that the test coverage should be expanded for.

Finally: please include a GIF showing this in action.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

This must happen after ๐Ÿ“Œ Fix the visually broken "image" component instance: use FileUriItem's computed `url` property, not the stored `value` property Active at least, and possibly more issues, to minimize disruption for in-progress MRs.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Almost ready! ๐Ÿ‘

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Thanks, @akhil babu! ๐Ÿ˜„

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

@lauriii was being trolled by Google Chrome โ€” he literally couldn't type anything anymore ๐Ÿ™ƒ So, making this change on his behalf ๐Ÿ˜„

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

One post-review thought: we need to verify that the state service remains in sync with the SDC discovery cache. Otherwise, we risk that the state key-value pair is deleted while the SDCs already are discovered. That'd result in zero incompatible SDCs getting listed, while we know that's not the case.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Definitely on the right track! ๐Ÿคฉ๐Ÿ‘

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

At this stage I'm assuming it's debug css that has been accidentally committed. If it's meant to be there we can add it back into the hero but properly scoped ๐Ÿ˜„

๐Ÿ˜„

@jessebaker was just pointing that out!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Reproduced the last failure:

Drupal\Core\Render\Component\Exception\InvalidComponentException: [image.src] Does not match the regex pattern ^(/|http).*\.(png|gif|jpg|jpeg|webp)(\?.*)?(#.*)?$ in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 203 of core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).

Investigating ๐Ÿ•ต๏ธ

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

This stopped failing!

So, postponing that on the upstream issue landing.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

FYI, per ๐Ÿ› [PP-1] Can't toggle boolean prop back to true after changing to false Postponed , @kristen pol created these related issues, which I suspect ought to be child issues of this one:

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

I already reported this over at #3463842-6: [META] Redux sync on ALL prop types, not just ones with a single [value] property โ†’ , where @bnjmnm is devising next steps. Linking this there.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

#5++

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Thanks for creating this, @danielveza! ๐Ÿ™

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

There is an accumulating overhead for every additional event subscriber. This affects 100% of requests/responses, and if itโ€™s only for architectural purity, then itโ€™s not worth the overhead IMHO. But I don't feel strongly about this, plus at this early stage, that's essentially premature optimization.

So, deferring to the owner of this part of the codebase, @traviscarden, and RTBC'ing ๐Ÿ˜Š๐Ÿ‘

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ‘ Thanks, @balintbrews!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Needs a new round of feedback from @lauriii.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

@danielveza That's a relief! ๐Ÿ˜ฎโ€๐Ÿ’จ

I'd like to avoid future devs having pain when trying to track down what is most likely a quite annoying performance regression to figure out.

That's totally fair.

Probably the softer approach for XB rather than the feature flag that removes it is a hook_requirements implementation that runs if JOSN:API is also enabled. I'll open a ticket.

I was going to suggest docs but โ€ฆ this is far better! ๐Ÿ‘๐Ÿคฉ Thank you! ๐Ÿ™

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Precise STR would help ๐Ÿ™ Better yet, when you reproduce it, look in the network inspector at the request body for the last request to /api/preview/โ€ฆ. Find the entry for image and you'll see exactly what the value is that is being sent for it. That'll determine where the root cause lies: back or front end.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

We can make the page hierarchy (or layers) feel more robust by adjusting the styles in a way where elements are not moving around between state changes. We should consider both focus / active styles, as well as drag and drop.

Is this enough information for you to act on this, @balintbrews or @jessebaker?

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

First unbroke the MR, by resolving a conflict: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

Then paired with @bnjmnm, to figure out why things were failing in the build for that commit. Turns out that we're hitting a new edge case for input--SOMETHING.html.twig. We discussed and agreed that it'd make more sense for those two files to be omitted and deferred to follow-up issues that are more tightly scoped โ€” they'll be child issues of ๐ŸŒฑ [META] Redux sync on ALL prop types, not just ones with a single [value] property Active .

I trust @bnjmnm's judgment which parts of this MR to land and which parts are more reasonable and feasible to land in a follow-up :)

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Great catch/call about uri vs uri-reference, @tedbow!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ“Œ Also validate request bodies against the OpenAPI spec Downport landed. I don't see how this particular case could occur anymore.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

(Not a blocker anymore thanks to @bnjmnm's analysis on #3455975.)

This reminds me conceptually of ๐Ÿ“Œ Stabilize FunctionalJavascript testing AJAX: add ::assertExpectedAjaxRequest() Fixed โ€” I wonder if we can do something similar here? Any inappropriate "wait" would then explicitly fail.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿฅณ โ€” thank you, much appreciated, @omkar-pd! ๐Ÿ‘

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

@q0rban: Yay, thank you! ๐Ÿš€

We currently do this manually when developing XB:

# See it in action + recommended development environment
1. Drupal 11 (preferably a git clone, for git archeology โ€” 10.3 will work too).
2. `composer require drush/drush`
3. `drush si standard`
4. `drush pm:install experience_builder`
5. Browse to `/node/add/article` โ€” you'll see a `๐Ÿช„ XB Demo โœจ` field. Don't touch that โ€” just enter a title for the article and hit save: a component is rendered using the article title ๐Ÿค“
6. If you're curious: look at the code, step through it with a debugger, and join us!
7. If you want to run *all* tests locally, including the OpenAPI spec one: `composer require league/openapi-psr7-validator webflo/drupal-finder devizzent/cebe-php-openapi --dev`

โ€” /CONTRIBUTING.md

But we also have and end-to-end test that literally performs those steps, added months ago ( โœจ [MR Only] Edit any component prop, powered by a new FieldForComponentSuggester service, which will power the JS UI Fixed ) in the super early XB days, but still serving its purpose: \Drupal\Tests\experience_builder\Functional\EndToEndDemoIntegrationTest::test()

And the Cypress end-to-end tests (similar to Nightwatch, but better) perform similar steps:

    $field_definitions = \Drupal::service('entity_field.manager')->getFieldDefinitions('node', 'article');
    $image_field_sample_value = ImageItem::generateSampleValue($field_definitions['field_hero']);
    $node = Node::create([
      'type' => 'article',
      'title' => 'XB Needs This For The Time Being',
      'field_hero' => $image_field_sample_value,
    ]);
    $node->save();

โ€” \Drupal\Tests\experience_builder\TestSite\XBTestSetup::setup() (@abhisekmazumdar in #4: look at that ๐Ÿ˜Š)

Proposal: if you can make this work to the point where it installs the Standard install profile, creates the first article node (and perhaps fails for now) and then navigates to /node/1, I'm happy to fill in the few missing pieces. ๐Ÿ˜Š๐Ÿ˜‡

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ› Retain the placement of components within the preview when inserting a component Active landed, let's land this next? ๐Ÿ˜„

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ› Retain the placement of components within the preview when inserting a component Active landed!

Is this unblocked now, or is it blocked on ๐Ÿ“Œ Improve UX of adding new sections Needs review ?

Assigning to @balintbrews to find out ๐Ÿ˜‡

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

#21:

  1. @traviscarden has AFAIK started on ๐Ÿ› OpenAPI validation errors must be provided as a JSON response Active ๐Ÿ‘
  2. I cannot find this follow-up yet among https://www.drupal.org/project/issues/search/experience_builder?issue_ta... โ†’
  3. @traviscarden did this: ๐Ÿ“Œ Add TravisCarden to `CODEOWNERS` for OpenAPI Fixed ๐Ÿš€
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

WFM in general, with one exception:

/xb-api/v1/{resources} [ /{resourceId} [ /{subCollections} [ /{resourceId} ] ] ]

-1 to versioning the API at this time, because that implies it's a public API. It is not. Defining a public API for this that we provide BC for is definitely not in scope.

If you really want to have it, then let's use:

/xb-api/v0/{resources} [ /{resourceId} [ /{subCollections} [ /{resourceId} ] ] ]

(with v0 matching XB's major version: 0.x)

P.S.: I do not understand why

We should also avoid /xb/ if we're going to use that path for administrative UI routes.

matters? /xb-api/โ€ฆ vs /xb/api/โ€ฆ are essentially the same?

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Lots of MRs landed, this now conflicts.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Fixed issues that should help y'all:

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

โœจ Implement the concept of sections within the client Fixed landed ๐Ÿšข

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Great! TIL about https://www.drupal.org/project/layout_builder_iframe_modal โ†’ ๐Ÿ˜„

Then the next steps are pretty clear โ€” updated issue summary accordingly.

Assigning to @bnjmnm for review/confirmation of @lauriii's proposal.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Thanks!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Can you please update \Drupal\Tests\experience_builder\TestSite\XBTestSetup to make the same "test with an image that contains a space" changes that https://git.drupalcode.org/project/experience_builder/-/merge_requests/189 introduced for ๐Ÿ› `{type: string, format: uri}` disallows image URLs containing spaces, `uri` data type stores + returns invalid URIs! Fixed ? ๐Ÿ™

That should be able to reproduce it then!

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Ideally we could outsource the styling completely to the admin theme so that we don't have to own this within the XB.

I don't see how this is realistic/feasible, because those admin themes will understandably style dialogs on the back-end only, not in the front-end (system.theme:default) theme.

That's an unsolved problem in Drupal core too: #2195695: Admin UIs on the front-end are difficult to theme โ†’ .

I don't understand why are we loading the media library within the XB application instead of loading it inside an iframe that would have all the required styles from the admin theme?

No idea โ€” that's the part of #3454173 that @bnjmnm handled, so assigning to him for answering that :) I suspect it's simply "because getting it to work at all was the only goal for #3454173" ๐Ÿคž

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ‘๐Ÿšข

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

FYI: @f.mazeikis landed ๐Ÿ› Component config entities are incomplete: missing entries for optional props, causing errors in ComponentPropsForm Needs work earlier today, he's now pushing this forward ๐Ÿ‘

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

#6.4 WFM!

Do you agree with

Can we get started with a E2E test that fails on failing to find the required HTML attribute of the test_REQUIRED_string prop on the all-props test component? ๐Ÿ™

?

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

The back-end side LGTM now :)

AFAICT

_Nice to have:_ Refactor `layoutModelSlice.ts::replaceUUIDsAndUpdateModel()` and `layoutModelSlice.ts::insertMultipleNodes()`, make them unit testable, then write unit tests

is not yet addressed.

That's fine: it's a nice-to-have. Can we extract a follow-up issue for that, and have a less experienced team member take that on? ๐Ÿ™

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

@jessebaker reported the same bug at ๐Ÿ› It should not be possible to drag a component from one preview into the other Closed: duplicate . Closed that as a duplicate.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

This is a duplicate of ๐Ÿ› Block dragging components from the desktop view to the mobile view Needs review .

Transferring credit.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ‘๐Ÿ™

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

๐Ÿ“Œ Create a component for testing form backend + frontend integration Active landed weeks ago ๐Ÿ‘

@bnjmnm, FYI: this has been added to ๐ŸŒฑ Milestone 0.1.0: Experience Builder Demo Active not to solve all the things, but to address at minimum โœจ Allow components to use textarea in favor of input Needs review โ€” and perhaps also type: boolean (the latter is AFAICT a blocker in ๐Ÿ“Œ Component props form: map textarea, bool, and select elements to React components Fixed being reported by Utkarsh).

(i.e. make at least those 2 field widgets work, just like ๐Ÿ› Prop select lists don't affect the component Fixed made <select> work.)

It makes sense to me to prioritize this issue now, so that @bnjmnm can convert it to a meta issue where he outlines his plan for scaling this to all Drupal core field widgets.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

With ๐Ÿ› Prop select lists don't affect the component Fixed in, I'm now 100% confident this works.

So writing a passing tests should be easy.

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

One unaddressed nit, and that last commit introduced one more ๐Ÿ˜…

๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Merged in upstream, looks like more tests than before are failing.

Production build 0.71.5 2024