🇩🇪Germany @stefan.korn

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

stefan.korn → created an issue.

@jannakha: Please see screenshots

Claro:

Bootstrap5 without this MR (notice both Save and Preview are primary):

Bootstrap5 with this MR (Preview now Secondary):

This will possibly conflict with some other MRs I have created, because of using preprocess_form_element and preprocess_input in these MRs too. If acceptable the MRs would need to be processed on after another and need to be rerolled.

Screenshot of regular textfield and textfield with floating label:

Screenshot of regular textfield and textfield with floating label in focus:

Code is

    $form['text'] = [
      '#type' => 'textfield',
      '#title' => 'Textfield',
    ];

    $form['text_float'] = [
      '#type' => 'textfield',
      '#title' => 'Textfield',
      '#attributes' => [
        'placeholder' => 'Password',
        'floating_label' => 1
      ]
    ];

stefan.korn → created an issue.

added styling for media type tabs

screenshot:

stefan.korn → created an issue.

stefan.korn → created an issue.

Regarding Task #1 I copied over the whole variable twig comment from stable9. This has some additional small changes beside adding description_display part.

see updated MR

@jannakha: thanks for reviewing this.

Regarding css/components/form.css: We could do the changes instead in scss/drupal/_forms.scss, but this does have some caveats too I suppose. One would need to overwrite the CSS from form.css in forms.scss, maybe something like margin-left: inherit;. While this is possible, seems not to be the cleanest way.

In addition this would somehow still depend on form.css. If for example the CSS selectors are changed in form.css, the fix in forms.scss might not apply anymore. This is kind of a hidden dependency.

So maybe think about if you might not still prefer the way via form.css. Maybe one could add a line with comment like "removed unnecessary margin for description" and then if you incorporate upstream changes from starterkit you would see this line in a git diff and can keep track of the change this way. This would of course not work if you incorporate the upstream changes automatically without reviewing. In that case we would probably need to do it elsewhere like in forms.scss

@jannakha: True, that issue exists in the starterkit theme as well. Did not expect that, though I spotted a difference in form-element template template in 💬 form element description does not get description class if description display is set to before Active already.

Wondering why starterkit uses different template from its base theme stable9 at all here ...

Okay, I think there have been some unrelated changes to CSS in the MR introduced by changing/updating the node packages.

I think, this should not be part of this issue, but in case it is needed be solved in a separate issue.

There were some really strange changes to the CSS like

--bs-primary-text-emphasis: rgb(5.2, 44, 101.2);

This results from latest changes in SASS, see https://sass-lang.com/documentation/breaking-changes/color-functions/ and using 1.79.x for sass would require changes in handling colors. But this is surely out of scope for this issue. More a general build issue for bootstrap5 theme.

So imho the build process should be done with sass version 1.54.0 as current package-lock.json states.

I therefore did the build process with sass 1.54.0 and reverted changes that were introduced by building with newer sass version (1.79.1).

I can confirm the issue and the MR is greatly helping to fix this.

I even think that the modal size and tab styling is okay now.

stefan.korn → created an issue.

No offend to maintainers, but this is a perfect example where the ever growing number of (probably mostly automated) "code style" fixings get worse by breaking an essential function of the module. I am a module maintainer myself and imho it's like kind of spam nowadays how these issues are created at mass without having real value in improving the module.

And specifically the rule "All functions defined in a module file must be prefixed with the module's name" has potential for trouble causing BC breaks. This should not be lightly used in such automated fixes.

And once again, full respect and thanks to maintainers of devel_debug_log, I use it very often and it is very helpful.

stefan.korn → changed the visibility of the branch 3474980-align-default-button to hidden.

stefan.korn → created an issue.

@cilefen: Thanks for quickly looking into this. 🐛 Prefix is shown before description Active is not touching the related part (see this vs that in issue description), so it would not solve this issue and thus is probably not related, beside that it is touching the same template.

stefan.korn → created an issue. See original summary → .

stefan.korn → changed the visibility of the branch 3474642-fieldset-template-should to hidden.

stefan.korn → created an issue.

The action plugin has been moved to Action module in Drupal 10.3, see https://www.drupal.org/node/3424506 →

this patch should apply to 10.3

that said, for Drupal 11 the patch needs to be ported to the then separated action module: https://www.drupal.org/project/action →

committed a change that adds support for YT shorts.