Een hart voor de Drupal Community

Brecht Ceyssens added a comment on drupal.org: Too few arguments to function Drupal\image\Controller\ImageStyleDownloadController::deliver()

Problem/Motivation

Related to #3344334: Settings Tray does not close on placement of block, when using layout_builder_browser module, instead of the default settings tray, the browser window should also be closed upon placement of a block from within the iframe.

Problem/Motivation

I would like to become a co-maintainer of this module to merge the Drupal 11 compatibility patch along with any appropriate RTBC issues and create a D11-compatible release.

You can view my profile to see my large history of work.

Proposed resolution

Review my profile and, if approved, add me as co-maintainer with appropriate permissions to merge code, manage issues, and create releases.

Remaining tasks

Add me as co-maintainer for this module.

I was looking for this functionality as well. We have a website that has pricelists 'linked' to a value on the user entity.

We import our pricelists from an ERP, and we've worked around this issue by hooking onto the user & pricelist insert/update events.

Pricelist on insert/update: - Find all users with the custom value and attach them to the customers field User on insert/update: - Remove the user from pricelist(s) with the old value (if applicable) and add to pricelist(s) with the new value

The main problem here is that, if no users have the value of that pricelist, the pricelist applies to EVERY user. That's something that conditions would have easily solved. In our use case, we've solved this by creating a 'dummy' user and if no other users apply for the pricelist, we attach the 'dummy' user to the pricelist.

Configuration/development wise this would have been much faster and cleaner if we had conditions, but I understand the argument of speed is a factor here too. I'll set this as postponed, it's not and hasn't been really an active issue and if the need arises later on it can be reopened.

Just tested the new patch on 4.2.2, it works great.

Thanks a lot!

Fons added a comment on drupal.org: Fix the issues reported by PHP_CodeSniffer

Ah.. my bad, you're absolutely right. I overlooked the default value of the class property.

I'm not sure if a backport to 1.x is still necessary, I suppose it depends on how long 8.x-1.x will remain supported? If it will, then maybe it would be good to have that fix there too 😇

Hi @mably, apologies, I hadn't seen that!

Although if I'm not mistaken that doesn't entirely fix the issue I was experiencing, because the encoder will still not strip tags or trim strings unless a setting is specifically provided (whereas before it would strip/trim by default).

With the fix currently in the 2.x branch, the warning message is gone, but if the encoder is used in Views Data Export it'll still let any and all HTML through (without the possibility of configuring it otherwise 😞).

It would be easily solved if encoders allowed configuration forms, but since (I think?) they don't, would it be preferred to keep the old functionality intact?

I've created an MR that adds default values. This preserves the behavior pre-#3108301.

Problem/Motivation

The issue #3108301 has caused a regression for me. When using a views data export (so the 'data export' style, not the excel export style), and combining that with the format "xlsx" (using the XlsxEncoder from this module), values aren't trimmed or stripped of html tags. This happens because the last 2 settings below aren't provided with any values (so also no default values).

// src/Encoder/Xls.php Lines#386-399
protected function setSettings(array $settings) {
  // Temporary fix this until it would be fixed at the views_data_export.
  if (isset($settings['xls_format'])) {
    if ($settings['xls_format'] == XlsSerializationConstants::EXCEL_2007_FORMAT) {
      $settings['xls_format'] = 'Xlsx';
    }
    if ($settings['xls_format'] == XlsSerializationConstants::EXCEL_5_FORMAT) {
      $settings['xls_format'] = 'Xls';
    }
    $this->xlsFormat = $settings['xls_format'];
  }
  $this->stripTags = $settings['strip_tags'];    // <----
  $this->trimValues = $settings['trim'];           // <----
}

Steps to reproduce

• Create a data export display (NOT excel export)
• Select .xls(x) as your request format
• Add some fields
• Result: some fields will be formatted as HTML, not stripped from their tags

Proposed resolution

Perhaps we should provide default values to these settings, to preserve the functionality prior to #3108301?

randalv created an issue. See original summary.

Just checked if it was possible to alter the patch for it to work again.

Looks like it's more work than I anticipated due to https://www.drupal.org/project/simple_sitemap/issues/3484114

Hi @mably and @dejan0,

Thanks for your solutions! I've tested the MR, and it seems to work fine for me.

Thank you for the nice module! Experiencing a small issue though:

Problem/Motivation

Clicking the 'remove'-button on a filter of the BEF Links-type doesn't work as expected, in the url it changes the following: `filter_name[delta]=value` to `filter_name[value]=`

Steps to reproduce

Using the following modules: - better_exposed_filters - facets_exposed_filters - facets

Other facets (checkboxes BEF element) seem to work fine, but the links one suffers from this strange behavior.

Let me know if I can provide any more information, I haven't been able to figure this one out yet myself 😞

randalv created an issue. See original summary.

I haven't got the time to fully debug this in facets itself, but in my case it was solvable by rendering it in a custom block like below. I had to render in a custom block anyway because I wanted to add extra items before and after the exposed filters, so this was a quick fix for me.

/** @var \Drupal\views\Plugin\Block\ViewsExposedFilterBlock $plugin_block */
$plugin_block = $this->blockManager->createInstance('views_exposed_filter_block:events-overview');

// Make sure the facets are populated.
$view = $plugin_block->getViewExecutable();
$view->preExecute();
$view->execute();

return [
  // ...
  'filters' => $plugin_block->build(),
  // ...
];

PS. BetterExposedFilter links might link to the current page rather than the view page.. Something else to keep in mind 🥲

I'm also experiencing this issue in a new project of ours using Facets 3.0.0-beta4, exactly as described above.

On the view page itself, everything is fine. Anywhere else, all the facets are empty :(

I'll see if I can find anything, haven't had the time yet to debug.

Patch in #22 was working perfect with 4.2.1 but it doesn't apply anymore on 4.2.2 and the initial problem is introduced again.

Randal added a comment on drupal.org: Automated Drupal 11 compatibility fixes for msqrole

Drupal 11 compatibility is already implemented.

Problem/Motivation

Currently all the tools use the generic permission check 'administer site configuration'. However, this is a pretty broad permission, which does not allow for fine grained access control. For example we would consider giving this permission to clients, but would not want the extra tools provided by this module to be accessible by the same clients.

Proposed resolution

Provide a module-specific permission to access tools.

Remaining tasks

- Add a module-specific permission - Provide upgrade path

Thanks works great, using the patch in #8 for now.

Small nitpick: I noticed on mobile the 80% width makes small edits, on a phone for example, a bit hard.

I'll try to find a solution for this and come back with an updated patch when I can find the time.

Thanks!

Thanks works great, using the patch in #8 for now.

Hello thank you for your feedback, it does indeed steer me in the right direction.

The geocoding might overcomplicate it a bit.

I think I didn't take enough time to try to describe what I was looking for.

In the current module you can drop markers which is great but there is no out of the box way (as I can see) to add some context to a marker (which is shown in the tooltip).

For example it would be great to add markers in a neighbourhood to show locations like:

• Drop marker on the map and add a label "Post office" (that is shown in a tooltip when viewing the marker on the map)
• Drop marker on the map and add a label "School" (that is shown in a tooltip when viewing the marker on the map)
• Drop marker on the map and add a label "Bakery" (that is shown in a tooltip when viewing the marker on the map)
• Drop marker on the map and add a label "Coffee shop" (that is shown in a tooltip when viewing the marker on the map)

Thank you for the time to answer my questions and the work on this module.

Have a great day

Fons

Just tested it again and looks good to go.

Hi @mitsuko,

I'm not entirely certain what the case was anymore but when I built this module, I ran into some strange behaviour when the authenticated role wasn't present. I could test it again, but I figure it's not really important anymore.

In any case, the reason you didn't run into any such incidents is because with your patch, behind the scenes the module would still apply the authenticated role (even if you didn't select it) in the submit handler.

I think regarding the validation, we shouldn't worry about that anymore. Before it was deliberately made impossible to leave the role selectbox empty, so we had to double check in the validator to see if people didn't get around HTML validation and leave it empty anyway. But since we now want it to be left empty when required, we don't need that extra validation anymore.

Any extra roles that are added through the inspector will also fail the form, since Drupal automatically handles that as invalid input as it wasn't in the form element's `#options` array.

Thank you for your answers, I was indeed referring to the widget (and not the formatter) my bad.

As I can see now there is no default way to drop a marker on the map and add some text to the pop-up in the widget.

I guess the only way to get around this for me is to add an address field (that gets geocoded) and a text field field and render that with a view / code on the map myself.

Thank you for your feedback.

It's been released in 1.0.18. Thanks for your suggestion!

Hi @mitsuko,

I see your point, it would be useful to only masquerade as the authenticated role, however I think I'll fix it somewhat differently.

The authenticated role *should* always be selected, so removing it from the automatically selected roles might produce strange results. I'll just remove the 'required' parameter from the roles selector, leaving the field empty will result to only the authenticated role being enabled.

That should cover every use case.

I'll reopen this since the fix hasn't actually been released yet, nearly 2 years later.

@Maintainer, could you please release a new tagged version with this fix present? Thanks!