🇧🇪Belgium @svendecabooter

Looks good to me.

Tested this MR with the newly released 1.2.0-alpha1 release of the AI module, and everything works as expected.

Do we also need to document that file / image extraction will fail without the core patch 🐛 InvalidArgumentException: Invalid translation language (en) specified Active ?

svendecabooter → created an issue.

@valthebald
Can you also credit user "arwillame", because a chunk of code for Layout Builder translation support got taken from https://www.drupal.org/project/ai_translate_lb_asymmetric →

I tested the current MR, and this seems to work as intended.
Maybe the Drupal logo color changing according to the environment indicator settings could be an option?
Although from what I can see this is managed in CSS, so probably not very easy to make this configurable...

svendecabooter → created an issue.

Renamed issue for clarity.

Adding a simple patch file to just restore the canonical link, to fix this issue via Composer patching workflow (only recommended for websites not using the flattened site settings loader obviously), while awaiting a more robust solution in the next release.

I have added a new MR now, to attempt to fix this issue.
In the first commit, I just restore the canonical link for SiteSettingEntity.

In the second commit, I add a configuration setting to not render the site entity on its canonical link, but its edit form instead.
I have added an option in the config form to disable this.

I'm not really sure why site settings shouldn't have a canonical view actually... When you use the revision system of that entity (by deselecting the config option "Hide the administrative advanced options"), it would make sense that you can view the latest revision of the entity + earlier revisions, for comparison. They are only viewable for people with the "view published site setting entities" or "view unpublished site setting entities" permissions...

I'm not sure what the exact issue with the flattened site settings loader is exactly, that causes an infinite loop, but there might be another way to fix that, rather than messing too much with the default Drupal entity link templates...

Can someone check if my solution that renders the edit form instead, fixes the flattened site settings loader issue?
And could use some extra thoughts on whether the edit form rendering is a good idea to begin with...

We are encountering similar issues.
Site settings entities are no longer translatable after 📌 Remove the canonical route of the site setting entity type Active has been committed.

The translatability of an entity is checked by \Drupal\content_translation\ContentTranslationManager::isSupported().
This checks if there is a link template "drupal:content-translation-overview".

Looking into where this template is defined - \Drupal\content_translation\Hook\ContentTranslationHooks::entityTypeAlter, we see it is only defined after the following logical check:
if ($entity_type->hasLinkTemplate('canonical')) {}

Which is now no longer the case.

I think the safest solution would be to restore the canonical route, and perhaps just load the entity "edit" form for that route?

Tested this update, and it works well with ✨ Add text extractor plugins for image and link field types Active .

Tested exporting and importing with this MR checked out, together with the MR in default_content ( https://www.drupal.org/project/default_content/issues/3532596 ✨ Add support for custom_field module Active )

I was able to export and import all custom_field data, including entity references (tested with Media and Taxonomy term) + custom_field file and image properties.

Everything worked as expected.

@apmsooner

I have tested with the following setup:

- Drupal 10.5.1
- AI module with this MR branch checked out
- Core patch applied: https://www.drupal.org/project/drupal/issues/3386915#comment-16174131 🐛 InvalidArgumentException: Invalid translation language (en) specified Active
- Configured core content_translation and language modules.
- Enabled AI Translate module + set up "Entity reference translation" settings to include Content block / File / Content / Paragraph / Taxonomy term - max reference depth set to 5.
- Created content type with a bunch of fields (plain text, body, image, term reference) + Paragraphs module entity reference (paragraph had same type of fields added: plain text, body, image, term reference).
- Created a 2nd content type with a bunch of fields (plain text, body, image, term reference) + enabled Layout Builder. Created a block type with plain text field, body field, image field, term reference field

I encountered 2 issues:
- "Warning: Undefined array key 0" PHP warning - although functionality kept working. Fixed now
- Symmetric layout builder translation didn't work fully yet (since it was untested as stated above). Added fix for that as well now.

I'm unsure how to reproduce your behaviour that translation doesn't work at all.
Maybe there is some specific configuration that is interfering, since it requires a bit of setup to get all pieces in place...
Perhaps you can share a config export of your installation via Slack, so I can try to reproduce the unwanted behavior.

I have tested now with 2 different setups (D10.5 with Paragraphs & symmetric LB + D11.2 with asymmetric LB + custom_field) and can get everything translated as expected...

Done

Can this perhaps be added as a merge request?
That would make it easier for reviewing...

Agreed, these can all be disabled.
Specific websites can reactivate them via config_split if needed, but I don't see a particular reason for that currently.

It's a lot of config changes, but they seem sensible to me as a basis for the different sites, which can use config_split to update configuration specifically for their instance.

FYI: I have not tested the latest version of the MR yet with a regular Layout Builder setup (without layout_builder_at module installed).

Based on feedback above, I have also incorporated the logic of the ai_translate_lb_asymmetric module into the LbFieldExtractor.
This provides a single FieldTextExtractor for the Layout Builder field type, reducing conflicts between modules / plugins.
Based on the selected Layout Builder strategy for a given installation, it takes care of the proper AI translation.

This eliminates the use of the separate ai_translate_lb_asymmetric module. When this improvement gets committed, it seems fair to credit author(s) of that module as well.

Thanks for your work on this refactor / improvement @apmsooner.
I've been testing it out, and it works very well.

Tested with simple fields on a node, with a custom_field setup (with mentioned custom_field branch checkout), and a complex entity reference setup multiple levels deep.

I have added an additional commit for ReferenceFieldExtractor, that implements the logic I described in the MR comments.
It removes the needs for translatable_properties annotations, since it infers whether they are translatable or not, from the relevant (recursive) FieldTextExtractor plugin that gets called for that field.

Tested the same scenario's as described above after this change, and functionality kept working.
Let me know if you think this isn't a correct approach, then the commit can be reverted in the MR I guess.

I tried applying MR 103 on a Drupal 11.2 website, but am getting the following errors when setting up a current environment:

Drupal\Core\Render\Component\Exception\InvalidComponentException: [navigation:toolbar-button/icon] String value found, but an object is required. The provided value is: "development". in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 234 of /var/www/html/web/core/lib/Drupal/Core/Theme/Component/ComponentValidator.php)
#0 /var/www/html/web/core/lib/Drupal/Core/Template/ComponentsTwigExtension.php(124): Drupal\Core\Theme\Component\ComponentValidator->validateProps()
#1 /var/www/html/web/core/lib/Drupal/Core/Template/ComponentsTwigExtension.php(106): Drupal\Core\Template\ComponentsTwigExtension->doValidateProps()
#2 /var/www/html/vendor/twig/twig/src/Environment.php(420) : eval()'d code(48): Drupal\Core\Template\ComponentsTwigExtension->validateProps()
#3 /var/www/html/vendor/twig/twig/src/Template.php(402): __TwigTemplate_644fe1f4d3b3cc70a78b679130cdd653->doDisplay()
#4 /var/www/html/vendor/twig/twig/src/Environment.php(420) : eval()'d code(136): Twig\Template->yield()
#5 /var/www/html/vendor/twig/twig/src/Template.php(402): __TwigTemplate_4bb116eb700f12a2c42b5883d177516f->doDisplay()
#6 /var/www/html/vendor/twig/twig/src/Template.php(358): Twig\Template->yield()
#7 /var/www/html/vendor/twig/twig/src/Template.php(373): Twig\Template->display()
#8 /var/www/html/vendor/twig/twig/src/TemplateWrapper.php(51): Twig\Template->render()
#9 /var/www/html/web/core/themes/engines/twig/twig.engine(34): Twig\TemplateWrapper->render()
#10 /var/www/html/web/core/lib/Drupal/Core/Theme/ThemeManager.php(380): twig_render_template()

This change might need a warning that it depends on ai 1.2.x, and including it into custom_field might need to be timed together with the first (stable?) release in that branch...
I don't suppose it would be possible to both support 1.1.x-style FieldTextExtractor plugins, as well as 1.2.x-style...

@apmsooner:

Yeah I reckoned the module would need to be updated. Was just checking if there might be ways for backwards compatibility that I didn't think about. If this refactor goes into 1.2.x branch, I guess modules that extend this functionality, would need to create a separate branch for AI 1.2.x compatibility. There is also https://www.drupal.org/project/ai_translate_paragraph_asymetric → that provides a similar plugin.

The custom FieldTextExtractor plugins for the assymetric modules could probably also be included into the ai_translate module, rather than a separate module, but then the FieldTextExtractor plugin might need some extra logic to decide whether the plugin needs to kick in or not. E.g.:
- for asymmetric LB translation --> check if layout_builder_at module is enabled
- for asymmetric Paragraphs translation --> check if paragraphs_asymmetric_translation_widgets module is enabled
Or could that logic go into shouldExtract()? Guess not, since that's more about logic on field level, than general condition checking...
Now this logic is enforced by those separate ai_translate_* modules having the dependencies set in their info.yml file. If they got included in ai_translate itself, they might be activated where they shouldn't (i.e. if translation logic is symmetric instead)

@eelkeblok: I have merged issue 📌 Support OOP hooks Active , so this functionality could be implemented with OOP hooks in mind.
Haven't reviewed the code yet, but described functionality looks promising :)

Remaining issues have been resolved, and this has been added to 1.0.x branch.
@eelkeblok would be good if you could rebase your MR branch on 1.0.x and also add support for OOP hooks. Sorry for the extra work.

Dependency on custom_field removed, by explicitly checking if the module is enabled.

FYI:
At this point in time I would suggest to include the https://www.drupal.org/project/role_delegation → into Drupal CMS, rather than https://www.drupal.org/project/roleassign → , given that the former is more actively maintained, and has a wider install base.

This only if there is a need to include anything at all, as per phenaproxima's comment above.

I encountered the same problem recently. Not sure where this got broken, because it seemed to work before.
I can confirm the fix by bisonblue resolves the issue.
MR is against 1.2.x, but this should also be fixed in 1.1.x

Tried testing the MR, but the changes to the modules/ai_translate/src/FieldTextExtractorInterface.php break contrib FieldTextExtractor plugin implementations, such as in "ai_translate_lb_asymmetric" or "custom_field" module that I'm using.
There is no way we can make this work without breaking backwards compatibility?

I think a plugin system for detecting the current environment would be really helpful.
While a lot of people would opt to use URL matching to discover the current environment, I can see other methods being used:
- a config override in settings.php
- a setting in \Drupal::state()
- a system environment variable (retrieved via getenv()) - e.g. set by a CI script targetting a specific environment
- ...

Together with the patch to default_content mentioned above, this MR allows custom_field data to be exported by default_content, and imported by default_content import OR core's DefaultContent API, used by recipes.

Attached is a patch file for the current state of the MR, useful for Composer based patching workflows.

A patch has been added to default_content module, to make sure the Default Content exported YML files use the UUID identifiers of referenced entities within a custom_field field, rather than their (numeric) entity ID.
The patch can be found at ✨ Add support for custom_field module Active

On the import side of things, there is a patch for custom_field that makes this work: 📌 Support Drupal core DefaultContent API Active .

Created an MR that updates custom_field data in the Default Content export for referenced entities.

Original output without this MR:

field_my_custom_field:
    -
      label: 'Exported label'
      image: 21 # Some random file ID
      image__width: 1024
      image__height: 1024
      image__alt: Sunset
      image__title: ''
      file: 22 # Some random file ID
      term: 4 # Some random taxonomy term ID

After applying this MR. this becomes:

  field_my_custom_field:
    -
      label: 'Exported label'
      image: aa6340f0-0006-4f31-9f2d-5595f5cce028
      image__width: 1024
      image__height: 1024
      image__alt: Sunset
      image__title: ''
      file: c36ea199-cd17-4fe5-bec7-beef1ffb51b4
      term: 78ae1ee7-f2d4-4fc0-871e-1af92730df3b

The referenced UUIDs also get added as a dependency to the "_meta" definition.

svendecabooter → created an issue.

svendecabooter → created an issue.

Can you try if the patch in 📌 Set block entity values via FieldTextExtractor plugins Active resolves this issue for you?

A better fix for this issue is implemented in 📌 Set block entity values via FieldTextExtractor plugins Active

Tested with the changed that got committed into the ai_translate module.
Everything seems to work now!

MR added for this functionality.
Also attaching as a patch file, for Composer based patching workflows.

svendecabooter → created an issue.

Seems related to https://www.drupal.org/project/ai/issues/3529669 ✨ Handle link field translation as well Active

As reported in 🐛 [5.0.0-alpha3] Menu items do not get is-active class Active , this MR fixes the issue for me in ui_suite_daisyui for me, after adding some extra logic in the menu component of that theme.

I think having active menu items is pretty basic expected behaviour for a theme, so would be good to get this included sooner rather than later. Having to patch both a module and theme to get a visual indicator of the active menu item, isn't the best situation for getting adoption IMHO. Therefor I took the liberty to increase the priority of this issue. Feel free to lower priority if you disagree.

I fixed this by applying the patch in ✨ Add ability to track active menu items in menu source Active and adding the following to the ui_suite_daisyui:menu component:

(...)
  {% for item in items %}
  <li{{ (not item.url and loop.first) ? ' class="menu-title"' : '' }}>
    {% set item_attributes = item.attributes|default(create_attribute()) %}
    {# Start adding class for menu active trail #}
    {% if item.in_active_trail %}
      {% set item_attributes = item_attributes.addClass('menu-active') %}
    {% endif %}
    {# End adding class for menu active trail #}
    {% if item.url %}
(...)

Not sure if this logic can already be incorporated into ui_suite_daisyui, while waiting on its commit in ui_patterns.
It would be good to have this already in place by the time ui_patterns gets updated, but not sure what the policy is there...

I'm not sure if this is the right place for this feature request, but I would propose to add an alter hook to the getFeaturedPageActions() method.
In my case, I want a custom entity to have another "featured action", rather than the edit form.
Currently I don't see a way to achieve this.
IMHO core should provide sane defaults, but allow the option for contrib / custom module to change the default behaviour.

The logic in the MR now seems to work for most of the custom_field property types I have tested.
Still WIP:

• Link field is a challenge. The link itself gets translated currently, since it's a string. However source link https://google.com gets translated in my OpenAI instance to \nhttps://google.com\n, where the extra newline characters mess up the data. Ideally the link itself shouldn't go to translation...
• Image alt / title texts do not get translated yet, because their property is not a string DataType, but rather an entity reference...

Thanks for the commit!

We use Single Directory Components in our themes, so using a custom template override allows us to render the custom_field through a SDC component.
E.g.:

{{ include('themename:card', {
  label: item['label'],
  title: item['title'],
  subtitle: item['subtitle']
  body: item['body'],
}, only) }}

svendecabooter → created an issue.

As per conversation in #ai on Slack, moving this to 1.2.x branch, since it's a new / extra feature.

As per conversation in #ai on Slack, moving this to 1.2.x branch, since it's a new / extra feature.

I'm encountering the same issue when using Heroicons.

Could a new release be tagged with this improvement?

Patch file included for composer-based patching workflows.

OK I think this should be doable, but it would require the patch in ✨ Don't hardcode 'value' key for textual field translation Active to be committed first.
Not sure how fast that can happen, so this extra plugin probably won't be committed into custom_field, before that issue is resolved.
I'll try to get the MR here working in combination with that issue, and hopefully keep it as simple as possible.

Currently there is some logic that checks if a custom_field subfield / property is translatable or not.
But ideally that should check if that subfield is textual or not. Is there some mechanism in place to do that?
I guess just doing checks on CustomFieldType plugin instances? I.e. StringType & StringLongType?

svendecabooter → created an issue.

Created a merge request that abstracts out the hardcoded 'value' logic.
If the FieldTextExtractor plugin returns an array of type ['value' => 'Some text'] in their extract() method, everything should keep working as before. But if the FieldTextExtractor plugin returns more dynamic properties, e.g. via the custom_field module, this would now work.
Example data in the extract() method for a "custom_field" FieldTextExtractor would then be:

['question' => 'Some question'. 'answer' => 'Some answer']

The changed logic applies both to the translation via the UI (Controller class) or via Drush. Ideally, both should be refactored to abstract out logic that is the same in both implementations, to allow less code duplication, but that seems out of scope for this issue.

Thanks for giving this some thought / review.

Thanks for the feedback. I'm working on a fix. Might take me a few days to push some code though...

Linking issue in AI module, that complicates this functionality.

svendecabooter → created an issue.

svendecabooter → created an issue.

I applied the MR logic from ✨ Add ability to track active menu items in menu source Active .
Then in ui_suite_daisyui:menu component, I add a check:

{% if item.in_active_trail %}
      <!-- Try adding extra class here -->
{% endif %}

That doesn't seem to work... Maybe I'm missing some additional context

svendecabooter → changed the visibility of the branch 3528586-support-drupal-core to hidden.

OK it should be possible after all I think.

Say we can integrate with the contrib Default Content module, to produce output as follows:

field_logos:
    -
      logo:
        entity_type: 'media'
        entity: eab657e0-3320-41d2-ab4e-53a09b397758
      link: 'https://google.com'
      link__title: ''
      link__options: { }

(if not possible, worst case we need to document the exported YAML files have to be manually edited).

Then in CustomFieldEntityReference::setValue(). we can do something like:

  /**
   * {@inheritdoc}
   */
  public function setValue($value, $notify = TRUE): void {
    $entity = $value['entity'] ?? NULL;
    // START ADDED LOGIC.
    if (is_array($value)) {
      if ($value['entity_type'] && Uuid::isValid($value['entity'])) {
        $entity = \Drupal::entityTypeManager()->getStorage($value['entity_type'])->loadByProperties(['uuid' => $value['entity']]);
        $entity = !empty($entity) ? current($entity) : NULL;
      }
    }
    // END ADDED LOGIC.
    if ($entity instanceof EntityInterface) {
      ...
   }

I can spend some time to look further into this, but am currently time restrained, so just doing some braindump here.
Will try to elaborate on it next week.
Let me know if it would be a workable solution to extend the setValue() logic, for the use case of default content exports / imports.

Did some more digging, and it seems the datatype used in custom_field module is "CustomFieldEntityReference", which does not inherit from core "EntityReference" data type, so guess this will never work :'(

svendecabooter → created an issue.

1.2.2 has been released now

Thanks a lot!

Thanks - committed!

svendecabooter → created an issue.

Thanks for the fix, and sorry for the delay in committing.

Thanks for the fix.
Committed now.

Thanks for looking into this sarvjeetsingh.
It hadn't occurred to me that this could be the source of the issue.
Fix in MR resolves the issue for me.

Added MR that adds this functionality + a patch file for Composer-based patching workflows.
Thanks for considering merging this functionality.

svendecabooter → created an issue.

svendecabooter → created an issue.