🇩🇪Germany @stefan.korn

Shouldn't there still be at least a hint somewhere (README?) that the toolbar module is required if gin toolbar is used without the navigation module? Or if technically not required, than something like a hint the toolbar module enhances the functionality of gin toolbar as long as navigation module is not enabled.

Currently with a minimal installation and just enabling Gin Toolbar nothing seems to happen. Some background things like style injections may still happen, but the toolbar itself is not visible. The toolbar only gets visible after enabling the toolbar module.

And if

When navigation is enabled, the toolbar module should be disabled.

When navigation is disabled, the toolbar module should be enabled.

is valid, why does the warning about that is surpressed from 🐛 Status warning: Toolbar and Navigation modules are both installed Active ?

May well be, that I am misunderstanding something here, but am bit puzzled about this now.

stefan.korn → created an issue.

Interesting, wasn't this a security issue? Even more relevant than the last security issue for Config Pages imho, https://www.drupal.org/sa-contrib-2025-093 → ?

and credit to @umekikazuya should really be given on this, greatly deserved I suppose.

stefan.korn → created an issue.

@andileco: Thanks for your input. I fully agree about this:

So, I don't want to add fields where they don't do anything and the user thinks something is broken when in actuality the field was just for a different charting library.

At the moment I am just tweaking charts solely towards chartjs to make use of options chartjs provides. Due to a somewhat tight deadline on this, I was not able to get in depth to adress the issue you mentioned. Because checking all the libraries for what they can do against options provided by the module seems really a lot of work.

But many thanks for the hint about "addBaseSettingsElementsOptions()" just for the chartjs module. This seems very interesting and maybe this way I can go forward with modifications for chartjs in the first place and maybe later address other libraries too and do refactoring of configuration options like you mentioned for the "three dimensional chart" in another issue.

Okay, I (hopefully) replaced all old array syntax to new one in this file.

stefan.korn → created an issue.

Yes, setting based on library support would be the best.

And thanks for the info about the color changer and future plans.

From looking at the code only Chartjs and Google seem to use this (not checked if other libraries themself might support setting title font size, but have not been implemented in the module). Then yes, it is just like with the color changer. Bringing in an option for all libraries that is not available for all libraries. Though the 'title_font_size' looks like a global option. That is what made me think it is used for all libraries. But surely it is not that easy.

I am currently working with Chartjs and trying to provide an interface for the users that is aligned with what Chartjs can do. So I am biased in this way.

Your idea from 🐛 Color Changer - only valid for Highcharts? Active to have libraries indicate their possibilities and settings somehow is very interesting and would surely be a good solution.

As there are many options and many libraries this might be a rather tedious task, but makes sense of course.

stefan.korn → created an issue.

see MR for possible solution

I am not sure about the color changer setting in the default configuration at admin/config/content/charts.

This comes from here: https://git.drupalcode.org/project/charts/-/blob/5.1.x/src/Element/BaseS...

But I am not sure why it is separately injected there and not used from here as well: https://git.drupalcode.org/project/charts/-/blob/5.1.x/src/Element/BaseS... , which would be possible as far as I can see.

stefan.korn → created an issue.

totally agree on this.

updated the MR with latest HEAD.

for some background info why sequential headings matter beside that tools like Lighthouse are reporting, read this: https://ux.stackexchange.com/questions/49111/is-it-ok-to-use-h1-and-then...

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

stefan.korn → created an issue.

I am also experiencing this issue if the setting for "Allow users to change the default charting library" is unset.

It seems to me just changing the form element type from "value" to "hidden" in this case solves this issue (see MR).

But I a wondering whether this issue is newly arising, the underlying code for the setting does not have seem to have been changed recently and seems to be in place for quite a while.

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

reroll with latest head

reroll patch with latest head

The list that admin_toolbar_tools generate on admin/index is really cluttering the UI and the link titles are often confusing.

If you want to stop the output of admin_toolbar_tools before this actually is put on the site, you could use a Route Subscriber and override the core controller to exclude the links from admin_toolbar_tools.

Like so:

your_module.services.yml

services:
  your_module.route_subscriber:
    class: Drupal\your_module\Routing\RouteSubscriber
    tags:
      - { name: event_subscriber }

src/Routing/RouteSubscriber.php

<?php

namespace Drupal\your_module\Routing;

use Drupal\Core\Routing\RouteSubscriberBase;
use Symfony\Component\Routing\RouteCollection;

class RouteSubscriber extends RouteSubscriberBase {

  protected function alterRoutes(RouteCollection $collection) {
    if ($route_index = $collection->get('system.admin_index')) {
      $route_index->setDefault('_controller', '\Drupal\your_module\Controller\AdminController::index');
    }
  }

}

src/Controller/AdminController

<?php

namespace Drupal\your_module\Controller;

/*
 * hide the admin toolbar extra tools entries on admin/index as it clutters the UI.
 */
class AdminController extends \Drupal\system\Controller\AdminController {

  public function index() {
    $output = parent::index();
    if (isset($output['#menu_items']['Admin Toolbar Extra Tools'])) {
      unset($output['#menu_items']['Admin Toolbar Extra Tools']);
    }
    return $output;
  }
}

I am a bit confused about this issue.

Firstly I suppose the solution via hook_file_insert given by TO is not considered to be a patch for this module, but rather a separate solution that you could use in your own module. In that case you should not install exif_orientation module.

The solution is depending on imagick → module and does not work with ImageMagick → module.

If you use ImageMagick module and Image Effects → module the solution could be changed to this:

/**
 * Implements hook_file_insert().
 * Rotates an image to its EXIF Orientation.
 * ONLY WORKS WITH IMAGEMAGICK TOOLKIT AND IMAGE_EFFECTS module!
 */
function HOOK_file_insert(EntityInterface $file) {
  if ($file->getMimeType() == 'image/jpeg') {
    $image = \Drupal::service('image.factory')->get($file->getFileUri());
    if ($image->getToolkit()->apply("auto_orient")) {
      $image->save();
    }
  }
}

Btw:
I am not sure what's the deal about stripping the EXIF information here. I suppose this is not related to the solution given by TO, but refering to a general issue with exif_orientation module and maybe having multiple rotations. And I agree with tfranz that stripping exif information can be a legal issue and an issue in general. This is kind of data loss, since the exif information is stripped from the image itself forever. This is no solution in my point of view, at most a very bad workaround. An advantage of ImageMagick is that it keeps exif information on image styles, which GD does not. Stripping the exif information would render this advantage useless.

had some issues with the __destruct variant in a scenario where we use some custom code to start a migration via backend.

So providing an alternative solution without __destruct but inheriting nextSource() function to unlink the temp file there.

I do not say that this solutions is better than the other one, just in our custom setup it works better.

So leaving decision up to maintainers how to fix this.

One has to say, that this only happens when drupal's temporary directory is different from systems temporary directory (/tmp in most cases).

That is probably a reason why not all users are seeing this problem. But nonetheless this is a serious bug. If you heavily use the XML Plugin it can fill up the disk space.

Thanks, for checking.

I am closing this issue and will schedule a new release shortly.

Okay, I am closing this issue. If for some reason, you find yourself in absolute need to have separate permission, you may reopen.

Okay, I will be thinking about this a little more.

Maybe it would be best to allow to provide one's own sorting function via extending in your own module. I will need to implement a hook or plugin solution for this.

Another approach outside of DKAN RSS would maybe to control this via dataset schema yourself. Maybe one could introduce a property to the dataset schema, that holds the date as a "computed" value that uses for example issued as first option, modified as second and entity->updated as last resort.

Yes, this would add flexibility.

But on the other hand, is there really need to deny access to the RSS feed? Seems kind of an edge case, that one could maybe handle with custom code. If introducing an additional permission, this permission needs to be set initially. Adds a little config debt imho.

But if it is important for you and you see a use case, than I am not reluctant to add the permission.

Interesting thought: If maybe one have a closed portal that allows only some roles to access datasets (which needs contrib module or custom code though, like Node View Permissions), will DKAN RSS respect that? Probably yes, because at the end we are relying on retrieveAll() from DKAN metastore.

made the block category more descriptive, but Block should basically show up in the Block admin UI.

warning should be gone and date sorting fixed like mentioned above, also a small fix for the "desc" sort.

Thanks for pointing this out.

Since the pubDate is defined by a property of the dataset (issued by default) and this property is not necessarily required, it is possible that pubDate is empty. pubDate is also not a required property of an RSS feed. So there should be handling there in case pubDate is empty.

Question is how to handle an empty pubDate while sorting by date. The php DateTime is using "now" as default. So we could go with that as a default as well. But that seems not suitable, because the sort would change dynamically in time.

So I think, putting a dataset with empty pubDate at the end of the feed, is probably the best option.

That said, you can also arrange a different sort (or no sort) if you change the schema json. Removing the "_sort" part altogether would apply no sorting. In that case the sorting is probaby introduced by DKANs retrieveAll().

Or you can change the sort to another property of the feed, like "title", if you put the "_sort" part like this in the schema json:

  "_sort": {
    "title": "baseSortAsc"
  },

This would sort the datasets by title from A to Z.

Thanks for reviewing the module.

Did you use the dev-version of the module? The block is still in dev branch only. And the block is shown under Category "Custom". I think this can be changed to "DKAN RSS" or something.

Will provide a new version soon, together with changes from your other issues.

stefan.korn → created an issue.

Oh yes, it is resulting from faulty config. We only disabled the server via config (as config override), but not the indexes.

Hm, MR8 is not compatible with D11? Going to admin/config/system/switch_page_theme will result in WSOD. Should we hide that branch?

MR7 is compatible with D11, but the domain functionality → is silently removed from the module? At least it should be explained why this is done.

stefan.korn → created an issue.

patch from #2 not working with current HEAD.

providing updated patch

Thanks for pointing this out.

Pushed a commit that just trims double quotes from the search string before comparing for exact match.

Seems feasible to fix the problem.

Currently thinking if there are other syntax related characters that needs to be removed before comparing (Solr?). But probably the exact search with double quotes is most common.

Thanks for pointing to this module. Didn't know this module up to now, and probaby didn't have found it before I was creating this module ...

It's true, both modules seem to be very similar.

From quick look, I suppose required_menu_link has the features of "soft require" and "disabling the automatic title for the menu link" which menu_force doesn't have, while menu_force provides the option to "lock parent item" and supports "taxonomy" which required_menu_link does not.

So there are some differences, but I acknowledge the similarity.

Will check differences a little deeper and add to module description when I find the time.

The partial import still does not respect translations.

It is discussed here too: https://github.com/drush-ops/drush/issues/2069

--partial uses a very low level API and apparently bypasses a lot of the config event system ...

This is probably the reason why translations are not imported with partial.

As far as I can see, there is also no way to import a single translation config yml (yml from configdir/language/langcode/) within the backend at admin/config/development/configuration, which also points in the direction that "partial" import will not work with translations.

There is also given a workaround in the discussion mentioned above:

config-export into a temp dir
copy the contents of the --partial dir into the temp dir
config-import from the temp dir

@drunkenmonkey: Sorry, missed the hint about test and review.

I can confirm that it works for me as expected now with the patch, though I am probably only really testing the suffix/prefix part.

Regarding the other variables, I do not have a case to test on.

In principle, I think the "not empty" check is preventing ambiguousness and should be favored. Only with the result count I am in doubt if a value of "0" should be considered empty or is expected to be empty? But is it possible at all to have a result count "0" and appear in the autocomplete suggetions at all?

I suppose this addresses ✨ Allow configuring of manual checking per field Postponed: needs info as well (using a site wide setting instead of form widget setting though). Maybe maintainers should decide whether to close ✨ Allow configuring of manual checking per field Postponed: needs info now as a duplicate or not.

Ironically this interferes with 💬 Views: Separate row per webform field Needs review . I had my share in both issues ... seems you can not have everything ...

This does not play nicely together with 🐛 Webform submission view filter for multivalues Needs work .

providing MR as a proposal. If this is acceptable, than one would probably do the same for hoo_reverse_geocode_entity_field_coordinates too.

stefan.korn → created an issue.

Hi @danflanagan8, yes #states is really nice, but I always forget the syntax and need to regoogle it ;-)

We use file download for a image gallery, where we provide an option for users to download the image in full size. We use an image media for the images and editors can give a name different from the filename to the image media and we want to have the downloaded image named like the image media is named (if this differs from real filename). That works fine on almost any browser as far as I can tell and if it would not work for some browser it would take the real filename, which is also nice because it does not break the functionality.

stefan.korn → created an issue.

Good idea.

What do we need?

• config schema
• Global setting form (where to place)
• Change in BreadcrumbBuilder logic based on this setting
• Disable the Third Party Setting for individual vocabularies and give a hint if global setting is active.

Will try to implement this soon.

I was also hit by this change, in that we are not using the assumed role name "administrator" for the admin role.

@eiriksm pointed out, a role called administrator does not necessarily exist nor need it to be an admin role.

Proposing to change the check for an admin user like this:

diff --git a/src/AdministratorTrait.php b/src/AdministratorTrait.php
index 60821e8..5f5128a 100644
--- a/src/AdministratorTrait.php
+++ b/src/AdministratorTrait.php
@@ -25,9 +25,18 @@ trait AdministratorTrait {
     static $root_user = NULL;
 
     if (!$root_user) {
+      $admin_roles = $this->entityTypeManager()->getStorage('user_role')->loadByProperties(
+        [
+          'is_admin' => TRUE
+        ]
+      );
+      $admin_role = reset($admin_roles);
+      if (!$admin_role) {
+        throw new \RuntimeException('No admin role found');
+      }
       $query = $this->entityTypeManager()->getStorage('user')->getQuery();
       $ids = $query->condition('status', 1)
-        ->condition('roles', 'administrator')
+        ->condition('roles', $admin_role->id())
         ->accessCheck(FALSE)
         ->execute();
       $users = User::loadMultiple($ids);

Would you consider to change this? I can open a new issue with MR in that case.

Proposal by eiriksm to change to previous behavior and load user 1 if no admin user is found, might also be an option.

Regarding tests I am not too proficient. I would maybe need some input on this. I guess I fixed on of the failing tests, but probably one would have the tests not only fixed, but also targeting the new setting.

Kindly ask for review.

stefan.korn → created an issue.

Yes it's true, the latest changes were not reflected in the module description and README. I have updated README and module description now.

Thanks for pointing this out.

see MR for a possible fix.

So far I could not spot any problems or changes in functionality of the toggler, but the warnings and notices are now gone.

stefan.korn → created an issue.

Thanks for testing and reporting deprecation notice, should surely not give this warning.

I have committed a change in the dev branch.

stefan.korn → created an issue.

I am using this for a while now, so setting to "Needs review"

Released version 1.3 to fix this.

Hook for adding formatters, like so:

function hook_media_parent_entity_link_alter_formatters(&$formatters) {
  $formatters[] = 'blazy';
}

works with blazy as far as I can tell.

Feature is now in dev branch.

stefan.korn → created an issue.

@erwangel: thanks for reporting.

I suppose the issue was introduced with ✨ It works with private images, not working with thumbnails RTBC , thus it will come with version 1.2.

I have committed a fix to dev branch. Can you test with dev branch?

I will make a new release soon, as the issue is clear in limiting media parent entity link to image formatter only in version 1.2.

I did probably not test ✨ It works with private images, not working with thumbnails RTBC with responsive image formatter.

@fenstrat: Thanks for your input.

Regarding the media type styling I was unsure. Claro uses vertical tabs:

Bootstrap provides this for vertical nav:
https://getbootstrap.com/docs/5.3/components/navs-tabs/#vertical
But imho this is looking a bit to "sleek", especially there is no visual hint regarding the active state. So I decided to use vertical pills, like shown at the end of this paragraph in the Bootstrap documentation: https://getbootstrap.com/docs/5.3/components/navs-tabs/#javascript-behavior

It seems Bootstrap is currently not providing something exactly similar with its defaults to what Claro is using.

I confirm the issue and the MR resolves this issue.

Setting to RTBC.

stefan.korn → created an issue.

I did also notice similar problem with locked submission in webform 6.2.x. The process is as follows:

- Create a webform
- add an action that locks webform after submission
- use token in the confirmation text

The token is not evaluated in the confirmation page if the locking action is present, otherwise the token is evaluated.

It also depends on confirmation type as it seems:

It works if confirmation type is set to
- Message
- URL with message

It does not work with confirmation type set to
- Page
- Inline

Okay, seems like one can override the ckeditor5-styles from varbase_components module as follows:

define a library in your_theme.libraries.yml as follows

ckeditor5-styles:
  dependencies:
    - core/components.your_theme_components--root
    - core/components.your_theme_components--alert
    - core/components.your_theme_components--callout

and then in your_theme.info.yml add this to libraries_override:
varbase_components/ckeditor5-styles: your_theme/ckeditor5-styles

not sure if this is already pointed out somewhere.

I think there is an issue with this, when trying to override the root component ✨ Add a Root component as a Base, Contains root CSS3 Bootstrap variables to Varbase Components Fixed in your theme. The ckeditor5-styles do not respect the override and you cannot easily override ckeditor5-styles dependencies in your custom theme yourself ✨ Allow overriding/disabling base theme's ckeditor5-stylesheets value Active (I am not even sure that the workaround given there does actually work. did not get it to work at least).

So if the ckeditor5-styles dependencies are called you get the root.css from the theme and from varbase_components and the order of appearance does matter. In my case the root.css from varbase_components came after the one from the theme. And anyway it does not look correct that both are called.

starterkit was probably added with #3050384: Provide a starterkit theme in core → . Add I think the issue with description display was fixed with #2396145: Option #description_display for form element fieldset is not changing anything → , but that did not find its way back in starterkit supposedly.

Question is maybe why the fieldset template (and some other templates too) are in the starterkit at all, when they maybe could be delivered from base theme stable9 as well?

I created core issue 🐛 starterkit theme should use same fieldset template than stable9 Active