🇳🇱Netherlands @vinlaurens

Utrecht, The Netherlands
Account created on 10 July 2014, over 10 years ago
#

Recent comments

🇳🇱Netherlands vinlaurens Utrecht, The Netherlands

Having the same issue on my end. Any known solution to this?

🇳🇱Netherlands vinlaurens Utrecht, The Netherlands

Patch in #4 seems to fix the issue, but causes another problem; When you input a text in the exposed fulltext search filter that yields zero results and then try to input text that actually does yield results, the count in the search summary is never updated again.

The actual problem seems to be caused by the setting "Show a text when there are no results" in the Facets summary settings. When this is enabled, upon yielding zero results as a consequence of inputting a text yielding zero results, the whole summary-wrapper gets replaced causing the code in facets-views-ajax.js:126 (indexOf) to not being able to find the summary-wrapper.

When "Show a text when there are no results" is disabled, this error does not happen. So this only seems to occur in specific situations.

I have no time to write a patch right now, but maybe it will give someone some leads to look into this issue. If not, I can possibly supply a patch myself at a later time.

🇳🇱Netherlands vinlaurens Utrecht, The Netherlands

Unfortunately, I was able to reproduce this using a pipeline in Gitlab which does a fresh site install with drush.

Drupal version: 10.1.2
Google Analytics: 4.0.2
PHP: 8.1

🇳🇱Netherlands vinlaurens Utrecht, The Netherlands

Version 2.0.2 is now compatible with Drupal 10.

🇳🇱Netherlands vinlaurens Utrecht, The Netherlands

This can be reviewed again with the last code changes.

🇳🇱Netherlands vinlaurens Utrecht, The Netherlands

Hi apaderno,

Thanks for reviewing. I have amended the code due to some of your comments, for others, I have some questions.

None of the Drupal core modules delete their own configuration objects, which I take it means Drupal core code does that automatically.

Alright. I have removed this code and it indeed seems that the configuration is already deleted upon module uninstallation.

For exceptions, the function used to log them should be watchdog_exception(), or code similar to that should be used.

Isn't it cleaner to use dependency injection here? As far as I see this is merely a wrapper function that ends up doing a static call to Drupal::logger in the end. Is this part of the standard or merely a suggestion?

There is no need to use those properties, in a class that extends ConfigFormBase. It is sufficient to use its config() method.
It would also be better to use more than a word for the property description.

Good point. I have refactored the code to use the parent's class config() method.

It is not necessary to use the full namespace for the PHP functions in the root namespace.

According to my mentor, who taught me to define the namespace for PHP functions, there is a small performance win for defining the namespace of these functions. I have however not tested this myself.

Since Drupal 8 is a unsupported branch, it should be better to update the core requirements.

Updated. I have also tested this module against Drupal 10 and made necessary changes, which makes it now compatible with both 9.x and 10.x.

I have updated the comment blocks.

All changes are reflected in the 2.x branch.

Thanks for your efforts.

🇳🇱Netherlands vinlaurens Utrecht, The Netherlands

Thanks for reviewing. I have updated the code to comply with PHPCS coding standards.

🇳🇱Netherlands vinlaurens Utrecht, The Netherlands
🇳🇱Netherlands vinlaurens Utrecht, The Netherlands

Hi dvanbrenk,

Could you update to the latest version and see if this is still an issue? If so, please open a new issue in the queue.

Thanks!

Production build 0.71.5 2024