akhil babu → made their first commit to this issue’s fork.
Just saw similar test failures in another merged MR: https://git.drupalcode.org/project/experience_builder/-/jobs/4335852. So I am moving this issue to needs review
Few test are failing even after rebasing. But those are fine in local..🤨
akhil babu → made their first commit to this issue’s fork.
Added the tests as well. Thanks
Thanks for the review @wimleers. I have updated the code to camelCase. Changes are made to the drupalsettings.xb
values. Please review
akhil babu → changed the visibility of the branch 0.x to hidden.
I have added a config action 'setBlockSettings' for updating the block settings
config:
actions:
block.block.claro_primary_local_tasks:
setBlockSettings:
secondary: false
Please review. I will add the tests if the current implementation looks good.
I will try to add the config action
Drupal\Component\Render\FormattableMarkup::placeholderFormat
always escapes the special characters in the string if the placeholder passed is '@'. This happens every time, and then the render API properly renders the special characters. But not sure why it's not happening in this case (Maybe because the rendering happens in the decoupled UI, but the label in the component listing /admin/structure/component
is also double escaped). Should we use html_entity_decode
here?
This issue does not affect for SDC components.
This part of the code is causing the issue
experience_builder_block_alter() {
//
$component = Component::create([
//
'label' => (string) new TranslatableMarkup('@label block', ['@label' => $definition['admin_label']]),
//
]);
}
(string) new TranslatableMarkup('@label block', ['@label' => $definition['admin_label']])
returns the incorrect label for blocks
Created a new MR. This time I am using snake_case for all parameters as it was more simpler to implement 😬
akhil babu → changed the visibility of the branch 3501847-send-entity-keys to hidden.
Conflicts
Ready for review
Thanks @wimleers and @omkar, I have updated the PR to include both the js_componen and xb_asset_library entities. The same id error does not occur for pattern entities as Drupal\experience_builder\Entity\Pattern::generateId()
updates the id if an entity with same id exists.
Updated ApiConfigControllers::post() and added the tests. Please review
akhil babu → made their first commit to this issue’s fork.
Hmm.. in my local only the 'og:type' validation part of the test if failing. But here it's failing while opening the front page 🧐
Moving to 'Needs review' so the maintainers can provide their insights on #4.
I have added the tests but it's failing because the 'Content' meta tags are getting applied to the node set as the front page. Eg: For the front page, the og:type is configured as 'website' and for other pages it's cofigured as 'article'. After creating a node and setting it as the site's front page, the og:type is still appearing as 'article' when opening the front page. This happens only while running the tests. If I try this directly from the ui (Create a new node and set it as the front page) , front page tags are coming properly
Also, the front page meta tag config was using 'field_image' to get the image meta tags like image_src, og:image, etc. But the default homepage is a basic page, and the basic page content type has 'field_featured_image' instead of 'field_image'. I’m not sure if it was a mistake or was intentionally set (since 'field_image' field exists in the article content type), but I’ve updated it in this PR as well.
I will try to add the tests
kristen pol → credited akhil babu → .
Tried adding the xb field using FieldConfig::create
and FieldStorageConfig::loadByName
just now. Field gets added correctly, but could not set setDisplayConfigurable('form', FALSE) and setDisplayConfigurable('view', FALSE)
Updated to use camel casing
Please review
Updated the controller to send the entity type keys
akhil babu → made their first commit to this issue’s fork.
Thanks for the review @wimleers. I updated the field creation code and added a check to see if any of the bundles of a given entity type uses XB or not (ie, experience_builder.enabled is true in the third party setting). The field and it's storage will be create only if xb is enabled for any of the bundles
However, I am now getting Base table or view not found: 1146 Table 'db.node__field_xb' doesn't exist
error after enabling xb for any of the content types.
I compared this code with the previous version, where I hardcoded supported entity types and noticed that tables like tables like taxonomy_term__field_xb, node__field_xb were created during the installation of the XB module (A major bug!) wheras now its not happenig. But, unfortunately, the tables are not getting created after enabling xb for a bundle as well.
akhil babu → created an issue.
Updated the documentation. Moving to needs review
Uploading https://git.drupalcode.org/project/drupal/-/merge_requests/9429 as a patch to use in one of our projects, Thanks
akhil babu → made their first commit to this issue’s fork.
Thanks @wim leers, I have updated the code to enable xb for taxonomy terms.
If an entity supports xb, we display the 'Experience builder: {title}' link in the toolbar while viewing the entity. However, I’m not sure how this would work for entities like blocks, where there’s no canonical URL. Maybe we add another tab, like "Experience Builder," next to the existing tabs?
Having the same doubt for user entities as well. We display 'Use Experience builder' option in the account settings (/admin/config/people/accounts) and then render the 'Experience builder: {title}' link in toolbar for all the user canonical urls?
In MR 526, an additinal tab for Experience builder was added to the node type edit form, and the value was stored as a third party setting in the node.
But as per #21, we should provide this setting in other entity types as well like taxonomy terms, block etc. But these entities do not have any 'details' element in the settings form. Should we implement separate form alter hooks like node_form_alter, taxonomy_term_alter etc.. to display the XB setting for each entities?
Similerly, experience_builder.schema
should be also updated to define the XB thrid party setting for each entities.
Is there any better way to store this setting other than using third party setting in entity types?
I think this test is for the ApiComponentsController
https://git.drupalcode.org/project/experience_builder/-/blob/0.x/tests/s...
Thanks @wim leers. I have updated the PR. Few observations after adding the field programatically
- Unable to load the xb UI for the already existing nodes. It's only loading for the pages creating after adding the field.
- If xb field is disabled for a bundle after enabling, the pages that were created after enabling the xb field still display the XB UI.
- Getting the following error on existing pages, after enabling the xb field
AssertionError: assert($items->count() === 1) in assert() (line 27 of modules/contrib/experience_builder/src/Plugin/Field/FieldFormatter/NaiveComponentTreeFormatter.php).
I tried adding the field using hook_entity_bundle_field_info() as suggested in #7.
/**
* Implements hook_entity_field_storage_info().
*/
function experience_builder_entity_field_storage_info(EntityTypeInterface $entity_type) {
$fields = [];
$field_name = 'field_xb';
$is_xb_supported_entity = in_array($entity_type->id(), ['node']);
if ($is_xb_supported_entity) {
$fields[$field_name] = // Field storage definition
}
return $fields;
}
/**
* Implements hook_entity_bundle_field_info().
*
* Add the example_field field to appropriate entity bundles.
*/
function experience_builder_entity_bundle_field_info(EntityTypeInterface $entity_type, $bundle, array $base_field_definitions) {
$fields = [];
$bundle_entity_type = $entity_type->getBundleEntityType();
if (empty($bundle_entity_type)) {
return $fields;
}
/** @var \Drupal\Core\Config\Entity\ConfigEntityInterface|null $entity_type_definition */
$entity_type_definition = \Drupal::entityTypeManager()->getStorage($bundle_entity_type)->load($bundle);
if ($entity_type_definition?->getThirdPartySettings('experience_builder', 'enabled', FALSE)) {
$field_name = 'field_xb';
$field = FieldConfig::loadByName($entity_type->id(), $bundle, $field_name);
if (empty($field)) {
$fields[$field_name] = FieldConfig::create([
'field_storage' => // Field storage definitions
'bundle' => $bundle,
'label' => '🪄 XB ✨',
]);
}
}
return $fields;
}
The field gets added correctl, but it is still visible in the UI, under 'Manage field/Manage display/Manage form display'. So user could still update/delete the field. Is there any way to hide it from the UI?
Need maintainer feedback to see if the current approach is correct or not. Unassigning for now
Hi @wim leers, My plan was to
- Add the option to enable/disable XB in content type settings
- Store it in the third party settings
- Add the XB field programatically, if enabled for a content type
Here is a basic implementation https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
The logic to enable XB on article content type was moved to a sub moudle in [# 3498085]. I will try to use the same logic to enable the field on other content types.
PHPUnit tests were already available for these routes and I have updated them to validate the CSRF token
experience_builder.api.content.update
experience_builder.api.config.patch
experience_builder.api.config.delete
experience_builder.api.config.post
Couldn't find any PHPUnit tests for the other routes. The cypress tests should be updated for them, I belive
experience_builder.api.preview
experience_builder.api.log_error
experience_builder.api.publish_all
akhil babu → changed the visibility of the branch 3490087-controllers-performing-data to active.
akhil babu → changed the visibility of the branch 3490087-controllers-performing-data to hidden.
As suggested by @larowan, I have added a new method toArray() to the prop source classes
I tried adding a new view bulder for terms like this
Term.php
'view_builder' => TaxonomyTermViewBuilder::class,
TaxonomyTermViewBuilder.php
/**
* {@inheritdoc}
*/
public function view(EntityInterface $entity, $view_mode = 'full', $langcode = NULL) {
// Get all the nodes that use this term.
$tid = $entity->id();
$query = $this->database->select('taxonomy_index', 't')->fields('t', ['nid'])->condition('tid', $tid);
$nodes = $query->execute()->fetchCol();
// Render all the nodes in teaser view mode.
$build = [];
$view_builder = $this->entityTypeManager->getViewBuilder('node');
foreach ($nodes as $node) {
$node_entity = $this->entityTypeManager->getStorage('node')->load($node);
$build[] = $view_builder->view($node_entity, 'teaser');
}
return[
'#theme' => 'item_list',
'#items' => $build,
];
}
The nodes are listed correctly when "Views" is not installed. However, the listing gets duplicated after installing 'views'. This could be fixed by simply checking whether the module is installed or not. but I am not entirely sure if this is the best solution.
Let me give this a try
I think it's not possible to add placeholder to the sort widget as view applies the first sort option by default.
Also, it is not possible to add a placeholder to select2 facets from the UI. I added it in the following way
*/
function my_modul_preprocess_select(array &$variables) {
$facet_id = $variables['attributes']['data-drupal-facet-id'];
if (isset($variables['attributes']['data-select2-config'])) {
$select2_config = Json::decode($variables['attributes']['data-select2-config']);
$select2_config['placeholder']['text'] = t('New placeholder')
$variables['attributes']['data-select2-config'] = Json::encode($select2_config);
}
}
akhil babu → created an issue. See original summary → .
akhil babu → created an issue.
I have updated the front page template by following the same component structure in
📌
Create MR with SDDS Sections so XB demo can use MR patch
Needs review
.
This is how the front page looks right now
Thanks for the guidance @wim leers. I have updated the validator code. Drupal\Tests\experience_builder\Kernel\ComponentTest::testObsoleteStatusHandling
still fails. I am not sure if this test should be removed or updated. Moving to needs review for feedback.
kristen pol → credited akhil babu → .
I ran across this scenario while exploring the component listing in admin/structure/component
- By default, the experimental component (components/sdc_statuses/experimental) is listed in the component listing and is active.
- Change the status of the component from
experimental
toobsolete
in experimental.component.yml - The component gets disabled in the component listing (Expected)
- The component gets added in the 'Disabled components' section (admin/structure/component/status) with reason "Component has "obsolete" status" (Expected)
- Changed the status back to
experimental
in experimental.component.yml - The component will still be disabled in the component listing
- The component will still be present in the disabled components section with the same reason "Component has "obsolete" status" . Since the component is disabled, it should be listed under the 'Disabled components' section. But the reason is incorrect. I am not sure what reason to use in this case though. Already, there is a 'Manually disabled' reason. But that's also not quiet right for this scenario.
Added a validation constarint to check if 'status' is TRUE.
Now updating the tests
akhil babu → made their first commit to this issue’s fork.
Unassigning for now
Ive added a test for validating the previous revisions. The test creates 3 revisions for the node. The link prop is used in the second revision. So, the exception message should contain '2' as the revision id.
The test works fine in local. But for some reason, revision id is coming as '1' in the exception message in gitlab ci and the test is failing...
I tested MR 277 and the media library widget works fine with JS aggregation enabled
Just noting down the findings
- Heading
Should be aligned with the icon cards section
- Case Study (starshot-case-study2)
Image overlapped with the text as logo, paragraph, and data components are not added to their slots. The components also needs some top margin.
- CTA (starshot-cta2)
The HTML tags are rendered in front end ascontent_right
and content_right_bottom
props do not use the |raw
filter in template.
- Stats + Four column in slot + Stats card in each slot
Style is broken when this combination is used
I have created the front page with the given components.
Hero (starshot-hero2), Four column with icon cards, and Testimonial components render perfectly while the others have few styling issues
Couldn't figure out why the widget is not working with js aggregation enabled :( Maybe something to do with the order in which the js is executed?
Thanks @wim leers. Committed the changed
Thanks @wim leers, I've used the same logic in \Drupal\experience_builder\EventSubscriber\ApiMessageValidator::isProd()
to check if assertions are enabled.
- Should we change this to a common function? (
ApiMessageValidator::isProd()
) is a private method - Should the priority be changed to
REQUIREMENT_ERROR
if assertions are enabed?
Added the warning.
Image effects have only uuids. So to remove an effect, we should pass it's uuid to the config action.
I created a config action 'deleteImageEffect' which accepts the uuid of the effect to be removed, as a starting point. Moving this to 'needs review' for feedback. Haven't added any tests for now.
config:
actions:
image.style.large:
deleteImageEffect: 62f7b707-a0cc-459b-a8ee-1dd3eeb9b445
akhil babu → made their first commit to this issue’s fork.
Thanks @smustgrave. Alexpott added a comment in the MR to add a test to verify the valid config actions. Drupal\Core\Config\Action\ConfigActionManager::getShorthandActionIdsForEntityType gives the list of shorthand actions per entity type, but this method is protected. So @alexpot suggested to hardcode the exception message.
https://drupal.slack.com/archives/C1BMUQ9U6/p1724325139885199
akhil babu → created an issue.
kristen pol → credited akhil babu → .
Updated the test to validate the config action names.
The code in XB enforces the 'attribute' prop to be of type Drupal\Core\Template\Attribute
web/modules/contrib/experience_builder/src/PropShape.php
// TRICKY: `attributes` is a special case — it is kind of a reserved
// prop.
// @see \Drupal\sdc\Twig\TwigExtension::mergeAdditionalRenderContext()
// @see https://www.drupal.org/project/drupal/issues/3352063#comment-15277820
if ($prop_name === 'attributes') {
assert($prop_schema['type'][0] === Attribute::class);
continue;
}
So, If 'attribute' prop is added with any other type (like type: string for most of the SDDS components), XB UI will break.
Thanks @sea2709. The 'notify_new_comments' recipe was there in web/recipes. It's also getting recreated when I run composer install after deleting it.
I got these warnings during ddev start step
No composer.lock file present. Updating dependencies to latest instead of installing from lock file
Authentication required (gitlab.lakedrops.com). I just hit enter for username and password and got "TypeError]
rawurlencode(): Argument #1 ($string) must be of type string, null given" error
Thanks @thejimbirch. I have opened a new MR against core