Een hart voor de Drupal Community

Sven Decabooter added a comment on drupal.org: Add Set fetcher method

Sven Decabooter added a comment on drupal.org: Add Fetcher support for more then objects

Sven Decabooter added a comment on drupal.org: Document module API

Sven Decabooter added a comment on drupal.org: collectiveaccess_feeds_fetch_all_total doesn't work

Done

Sven Decabooter added a comment on drupal.org: Port Feeds mappers from D6 to D7

Sven Decabooter added a comment on drupal.org: AES encrypted passwords don't work with Features

Sven Decabooter added a comment on drupal.org: Document module API

Sven Decabooter added a comment on drupal.org: Collective access

Sven Decabooter added a comment on drupal.org: Mark CollectiveAccess Project as Unsupported

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.

Here's a patch that applies the change that works for us.

Hi,

Thanks for the issue, it already got us closer to a solution as well. However, I notice a small issue in the `commerce_checkout.schema.yml` file.

commerce_checkout_pane_configuration:
  type: mapping
  mapping:
    display_label:
      type: string
      label: 'Display label'
      translatable: true
    step:
      type: string
      label: 'Step'
    weight:
      type: integer
      label: 'Weight'
    wrapper_element:
      type: string
      label: 'Wrapper element'

I *think* the `display_label`'s type should be "label" rather than string. Currently, the display label is editable in the UI but not translatable. Not every plugin provides a display label from its attribute/annotation, and since it is editable in the UI it should also be translatable in the UI.

I'm not sure if the combination of `type: string` with `translatable: true` does anything, though? I checked the "config schema cheat sheet" PDF file and couldn't find anything.

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()

I have the same problem, can this be committed? Fix seems to be straightforward.

Then I can’t think of a case where you need to calc the height dynamically where it’s not 100% of the height for the sidebar.

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)

Sven Decabooter added a comment on drupal.org: Add schema validation

@eelkeblok: I have merged issue #3519842: Support OOP hooks, 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?