Account created on 29 March 2012, over 13 years ago
#

Merge Requests

More

Recent comments

🇳🇱Netherlands idebr

Changes for Drupal 11 are available in the 3.0 beta, but no stable release yet. I don't see any blocking issues, so this might just be an administrative task?

🇳🇱Netherlands idebr

The change works as expected, thanks!

The CountryPathDomainListBuilder is no longer used and can be removed

🇳🇱Netherlands idebr
🇳🇱Netherlands idebr

Thanks, composer.json is sufficient to manage the domain package version

🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

idebr made their first commit to this issue’s fork.

🇳🇱Netherlands idebr

The cspell job reports a number of unrecognized words, one of them is "certinaly". Let's fix the other words as well.

🇳🇱Netherlands idebr

Why would you add max-age 0 when expires is expired? It will not update until the config is changed.

🇳🇱Netherlands idebr

Two things are currently missing:

  1. The development branch is not listed on the project page, compare https://www.drupal.org/project/webform_iban_field vs https://www.drupal.org/project/role_test_accounts
  2. Version 2.x cannot be selected in the issue queue

Both things can be fixed with the instructions in #4

🇳🇱Netherlands idebr

Looks good now, thanks!

🇳🇱Netherlands idebr

https://www.drupal.org/project/webform_iban_field has a link 'Add new release'. Here you can select 2.x as a development branch

2.x-dev is not necessary and can be removed

🇳🇱Netherlands idebr

The test is fixed now

🇳🇱Netherlands idebr

The merge request implements the following changes:

  1. Added a gitlab-ci.yml template, see https://git.drupalcode.org/project/gitlab_templates/-/blob/main/gitlab-c...
  2. The test is placed in the tests directory, so it is picked up by gitlab CI
  3. Added the webform dependency to composer.json
  4. Fixed the order of arguments for assertEquals()
  5. Added a void return type to the test method
🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

The eslint pipeline job still reports lint errors, see https://git.drupalcode.org/issue/paragraphs-3138762/-/jobs/6543959

🇳🇱Netherlands idebr

This issue was fixed in 🐛 Export single simple configuration name has no empty value, resulting in confusing UI RTBC . I'll close this issue as a duplicate

🇳🇱Netherlands idebr

The indentation change for composer.json is incorrect: composer.json uses indent_size 4. See for reference https://git.drupalcode.org/project/drupal/-/blob/11.x/.editorconfig?ref_...

🇳🇱Netherlands idebr

Functionally this is working great! I'll open a 2.x branch so this module can implement breaking changes for existing installations.

Some remarks:

  1. Do we still need the TempStore for saving selectors? Since these are now determined through the commented entity and the field name
  2. Let's use the native Element scrollIntoView for the ajaxCommentsScrollToElement command, see for reference https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView and https://git.drupalcode.org/project/drupal/-/commit/e1c818c662f8c0c6a1dc8...
  3. Can the highlight part of ajaxCommentsScrollToElement be called with ajaxCommentsHighlight so each command has a unique responsibility?

Further cleanup can be done in followups.

🇳🇱Netherlands idebr

@jsacksick can you tag a patch release with this fix to prevent updated sites from displaying this error on the status page?

🇳🇱Netherlands idebr

Inline Entity Form uses the RenderArrayTool Library for this, see https://www.drupal.org/project/rat

Specifically, see https://git.drupalcode.org/project/rat/-/blob/1.0.x/README.md?ref_type=h...

🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

This is being fixed in 🐛 Country path not managed in domain navigation block Needs work

I'll close this issue as a duplicate, so we can focus our efforts in the related issue

🇳🇱Netherlands idebr

The merge request updates the ContentTranslationController to add the new translation to a clone of the entity in the route, so access checks for the language switch links pass.

🇳🇱Netherlands idebr
🇳🇱Netherlands idebr
🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

MR updated after 📌 Fix LongLineDeclaration in Functional tests Active was commited

🇳🇱Netherlands idebr

@catch I assume this issue can be marked as fixed? Its status is currently still 'RTBC'

🇳🇱Netherlands idebr

The merge request implements the following changes:

  1. The configuration is now saved through the #config target property, see https://www.drupal.org/node/3373502
  2. Validation is now done through config schema instead of form validation
  3. Cache invalidation is now done through a config listener instead of form submit
🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

@loze thanks for continuing to work on this. I'll do an in-depth review later this week.

🇳🇱Netherlands idebr

Not sure Drupal can actually do anything to prevent the double header being emitted.

It can, and it does (for X-Content-Type-Options), see https://git.drupalcode.org/project/drupal/-/blob/11.x/.htaccess?ref_type...

🇳🇱Netherlands idebr

This option is available through Search API's query option 'Skip item access checks', see #2470914: Add Views support for individual fields

🇳🇱Netherlands idebr

#62 is now available as a merge request

🇳🇱Netherlands idebr

idebr made their first commit to this issue’s fork.

🇳🇱Netherlands idebr

🐛 Submitting a comment in the non default language redirects you to the default language Needs work is now postponed on this issue.

Updated the issue summary from a question to a statement

🇳🇱Netherlands idebr

idebr made their first commit to this issue’s fork.

🇳🇱Netherlands idebr

idebr changed the visibility of the branch 10.2.x to hidden.

🇳🇱Netherlands idebr
  1. Follow-up is no longer applicable since the changed code was removed earlier.
  2. Removed remaining tasks from the issue summary as they are no longer applicable
  3. Added assertion to check the comment is visible on the page to prevent a regression in UX (see #69). This assertion will keep failing until 🐛 Comments created in translation are displayed only for admin role Needs work is fixed

As a result, this issue is now postponed on 🐛 Comments created in translation are displayed only for admin role Needs work

🇳🇱Netherlands idebr

This was suggested before #2908899: Wrong configuration schema for block_settings.label_display property and subsequently closed because it would be counter-productive for Add option for visually-hidden block titles Needs work

🇳🇱Netherlands idebr

idebr made their first commit to this issue’s fork.

🇳🇱Netherlands idebr

The merge request ensures the translator is set when translating documents, similar to translating text.

🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

The merge request implements the life cycle for translated documents

I'll look into the phpstan findings when 📌 Fix code quality issues Active is fixed

🇳🇱Netherlands idebr

idebr created an issue.

🇳🇱Netherlands idebr

Based on the issue summary, comments and related issues the immediate problem is limited to single select elements and client side validation. If I missed something, let me know and I can update the issue accordingly

Refactoring the _none value itself is being worked on in 📌 Convert '_none' option to a constant and deprecate form_select_options() Needs work

🇳🇱Netherlands idebr

idebr changed the visibility of the branch 1585930-options-module-uses to hidden.

🇳🇱Netherlands idebr

To minimize the BC break, I suggest we map the _none option to the Select #empty_option and set #empty_value to an empty string in a process callback.

This implementation allows existing options to remain unchanged; only client side logic is impacted (eg. JavaScript)

🇳🇱Netherlands idebr

DeeplTranslatorBatch:: getTranslationOptions() is a bit messy right now since the settings priority is currently not implemented (Job settings > Translator settings -> Translator plugin defaults). I filed a follow-up issue to start smoothing this out, see 📌 Provide sensible default settings Active

🇳🇱Netherlands idebr

The merge request adds default settings by implementing \Drupal\tmgmt\TranslatorPluginInterface::defaultSettings

🇳🇱Netherlands idebr

In the merge request additional values in the 'Translate content' form are saved in the Job entity.

🇳🇱Netherlands idebr

TMGMT supports translation of files on a conceptual level, see https://www.drupal.org/node/3392215

The mimetypes that should be translated can be selected at /admin/tmgmt/settings under 'File translation'

Assuming you have a working setup with nodes, you can add a 'File upload' field to the content type and check the option 'Users may translate this field' in the field settings.

🇳🇱Netherlands idebr
  1. Translatable files are now added as a separate batch operation per file, see \Drupal\tmgmt_deepl\DeeplTranslatorBatchInterface::translateDocumentOperation
  2. A document is translated by Deepl through \Drupal\tmgmt_deepl\DeeplTranslatorApiInterface::translateDocument
🇳🇱Netherlands idebr

MR works as expected.

Not sure what problem is indicated in #12 because of the double negative, but messages are still shown in the user interface. I'll leave at 'Needs review'

🇳🇱Netherlands idebr

TMGMT enables additional settings when a translator plugin includes "files = TRUE" in its annotation

  1. \Drupal\tmgmt_deepl\DeeplTranslatorApiInterface::translate is renamed to \Drupal\tmgmt_deepl\DeeplTranslatorApiInterface::translateText to match the deepl client method
  2. Introduced \Drupal\tmgmt_deepl\DeeplTranslatorApiInterface::translateDocument

I will continue to iterate on this issue next week.

🇳🇱Netherlands idebr

The current hook tmgmt_modules_installed implementation is fundamentally flawed: there is no way to actually set the "auto create" to false as the annotation does not support spaces in the key as I discovered in 📌 Opt out deprecated Translator plugins from auto create Active

tmgmt_modules_installed was added as part of the initial 8.x port. I suggest this hook is removed in favor of config/install files as is customary nowadays

🇳🇱Netherlands idebr

The merge request fixes test failures on phpunit: next minor (11.x-dev)

🇳🇱Netherlands idebr

The merge request implements constructor property promotion to reduce boilerplate

The failure reported in phpunit: next minor is unrelated. A separate issue is available at 📌 Fix test failures on phpunit: next minor (11.x-dev) Active

🇳🇱Netherlands idebr

Is this not what the 'visible' key is for?

visible: (optional) The default visibility of the element. Defaults to TRUE.

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

🇳🇱Netherlands idebr

The merge request fixes spelling of authentification, occured, interpunction, tmgt

🇳🇱Netherlands idebr

The merge request adds config schema for key.type.deepl_api_key

🇳🇱Netherlands idebr

Changes look good. However, there is still an open thread in the merge request that is possibly outdated?

🇳🇱Netherlands idebr

Ran into this issue as well after updating to beta3. The issue was fixed in the -dev version, but the change to does apply cleanly as a patch

#3538150-18: 502 Bad Gateway on Frontend After Update to beta 3 (from 2) mentions

And if everything is ok, I'll publish a 2.0.0-beta4 release.

A new beta version should prevent other sites from running into this issue, so a new release tag would be very welcome

🇳🇱Netherlands idebr

Berdir mentions in [3396165-25]:

I realized that we forgot about another generic plugin feature beside deriver, the provider. It isn't used a lot in core for obvious reasons, but it's a valid use case to implement a plugin if another module exists.

Was this ever implemented for plugin attributes? Running into this now at the Linkit conversion: 📌 Support attribute based plugin discovery Active

🇳🇱Netherlands idebr

The merge request fixes phpstan/phpcs/stylelint pipeline job findings

🇳🇱Netherlands idebr

The autocomplete icon in the CKEditor widget cannot be updated with this approach without duplicating css, so the approach in 🐛 Improve autocomplete UI Active is preferable

🇳🇱Netherlands idebr

Icon looks much better now, thanks!

🇳🇱Netherlands idebr

Merge conflict resolved after 📌 Use latest rules for postcss-nesting Active was committed.

🇳🇱Netherlands idebr

The WebformBreadcrumbBuilderTest @dataProvider is now a static method

🇳🇱Netherlands idebr

idebr made their first commit to this issue’s fork.

🇳🇱Netherlands idebr

https://git.drupalcode.org/project/varnish_purge/-/merge_requests/12/dif...

In the commit message the dependencies in the info.yml file are assumed to resolve automatically. This assumption is incorrect for this module.

🇳🇱Netherlands idebr

The rename of the info.yml file is out of scope for this issue

🇳🇱Netherlands idebr

In the DisplayFeedTest.php the description is only checked for html, but not the CDATA. Can you update the assertion to include the CDATA, since this allows the inner text to be html ?

🇳🇱Netherlands idebr

The proposed resolution is to add a composer.json. The MR does not match the proposed resolution.

Production build 0.71.5 2024