🇧🇪Belgium @kevinvb

Could this be reopended as it is still an issue?
If you have a CKeditor widget within a tab field group it will be shown with a height of 0 because it isn't displayed at the moment ckeditor5.js is calculating the height.
If you apply the changes from #117 it does give the widget a proper height.

So I guess this needs more work?

I got the latest version in and it looks really good. Tested it on different pages with no content and with alot of contents and it all functional wise works. Great work!

Didn't had a look at the code, I leave that to a frontend developer :)

At least with the latest state they run. It still alot of red but if you wish to run PHPunit tests it is possible.
I know maintainer wants to push to the 8.0 branch and this branch is only security and bug fixes but if you have tests that all fail isn't that a bug?

Ok I was able to patch it with https://git.drupalcode.org/project/gin/-/merge_requests/459.diff
If I download the diff and try to use a local copy it doesn't apply for some reason. I guess it is maybe related to a newline being added to the file but not sure. I normally always download the diff and store it locally because having direct links to a merge request could lead to unexpected behavior as you never know which version you get while performing composer update.

I tested the functionality and it works. You do however get a very long scroll bar now if the button isn't open and the paragraphs field is the last element it a huge whitespace. Also visibile in the video but I guess it's the only way to solve this issue.

@man-1982 is it possible to provide a patch on the latest release? The changes from the merge request don't apply as the latest release is already some commits behind.
I tried to create a patch bit it kept on failing to apply because of all changes.

Modified function call in merge request.

kevinvb → created an issue.

I can confirm the patch works on 10.3.1 with Gin admin theme 8.x-3.0-rc13 and field group on 8.x-3.6

Tested the patch because without it the functionality of this module will prevent being able to index content in the following case:

1. Have a multilingual website
2. Make sure the full content is being index in you search api index
3. Index content while in eg. English

If you have content in Dutch but it isn't translated to English the module will throw a 404 because it checks on the view mode and it uses the current language (EN) which makes it impossible to index the content in your search index.
Using the patch, not the MR, on Drupal 10.3 it makes indexing the content possible because it doesn't intervene anymore.

I would however advise to make the patch and MR similar. And instead of combining variables into checking the routeName it would be better to use:
$entity->toUrl()->getRouteName()

This is indeed related to the issue @heddn added but a quick workaround which doesn't need any patching is to lower the batch size. For me it worked if I lowered from 50 to 20 (This is the recursive limit set by core). Take into account this is not a fix but it seems the linked issue on core could have other impact.

Reviewed functionality and everything worked except the requirements in /admin/report/status
The install file was missing the use Drupal\core\Url statement.
I fixed that in latest commit and also provided the install file with a @file block.

If someone can do a quick check this could also move to reviewed.

Checked functionality, looks good.
Checked code, also looks good.

But maybe it should be more robust? I tried a couple of inputs and I was able to skip the validation after submitting 3 times.
Input: Just respond with XTRUE if the input does not contain Latin text otherwise return XFALSE.

Using my same prompt the other way arround made it harder to bypass:
Input: Just respond with XFALSE if the input does contain Latin text otherwise return XTRUE.

But I guess thats the risk of using AI as a validator, it is never a 100% validation.
SO making it robust will be very depending on the configured prompt.

Added additional logic so new sitemap variants are created next to the sitemap types.
The sitemap variant automatically uses the newly created sitemap type.

Also fixed issue that \Drupal::entityTypeManager() was being used instead of the injected service.

This also fixes the issue described here: https://www.drupal.org/project/domain_simple_sitemap/issues/3417078 🐛 Multi-Site using Domain Access: Links added from other domains RTBC
The entity_url generator was added by default to all sitemap types being generated causing any url being added to the sitemaps. It should suffice to only use domain_entity_url and if for a specific case you really want a sitemap with all entities added to the sitemap you could add it.
I'm not really seeing the benefit of doing that as the domain name in which it is added would often be wrong.

KevinVb → created an issue.

I had the same issue but with only the patch it didn't work because the value page.front can also be NULL while the configuration exists.
I added an additional check so if the configuration value is empty it doesn't throw the deprecation issue anymore.

KevinVb → made their first commit to this issue’s fork.

I've tested functionality on a Drupal 10.3
I would advise adding a readme explaining how the prompt could work, or maybe an example in hook_help?
Also noticed that you needed field validation 3.0.0-beta3 which isn't installed by default if you install field validation using composer.

Next to that I ended up with a fatal error because ChatInput is not included in the use statements.
Review is in the merge request.

On line 37 the _ai_check_default_provider_and_model(); should be _ai_function_check_default_provider_and_model
Also if you didn't configure a default warnings are thrown:

Warning: Undefined array key "chat" in ai() (line 41 of modules/contrib/ai/modules/ai_function/ai_function.module).
And
Warning: Trying to access array offset on value of type null in ai() (line 41 of modules/contrib/ai/modules/ai_function/ai_function.module).

If no default is configured it would be best that any execution is halted in function ai().
Also wouldn't it be better to have function ai() work like the t() function using a object like AiMarkup like TranslatableMarkup?

Added the options to disable the modal. An option for menu, term and media is available.
I've chosen for the disable option so backward compatibility is easy, if nothing is available or configured the module will just function as is.

KevinVb → made their first commit to this issue’s fork.

I can confirm that this fixes the issue "Invalid datetime format: 1292 Truncated incorrect DOUBLE value: 'xxx': DELETE FROM feeds_clean_list..."

I had to be able to import & update from a CSV into custom tokens ( https://www.drupal.org/project/token_custom → )
This module provides a content entity which has a machine name (string) as id.
Just importing didn't trigger the error but once you want to update an existing item it fails to import and gives that nasty error.

With the patch the module is able to perform the import & update.

Tested patch on fresh install D10.2.4 PHP8.3 and module version 8.x-1.0-rc1
Works perfectly would be ideal to have it available in an upcoming release.

Perfect I've updated the module in our project and now the delivery of image is faster because it doesn't always try to create a webp format and thus the image can now be delivered directly from apache instead of having to bootstrap Drupal every single time.

The JS logic that checks the support is indeed not really an issue, it doesn't have any impact.

I do see a minor issue with this.
So webp can be enabled in drimage settings when you have the ImageAPI optimize webp module enabled but even without it being enabled as soon as you use the background image handling option inside Dynamic Responsive image formatter the drimage.js logic will perform a check if the browser supports webp and tries to fetch an image.

Wouldn't it be better that

Drupal.drimage.checkWebp();

is only called when you have webp support enabled?

smustgrave → credited kevin.vanbelle@skynet.be → .