🇳🇿New Zealand @John Pitcairn

Account created on 2 February 2009, over 15 years ago
#

Recent comments

🇳🇿New Zealand John Pitcairn

@Wim Leers:

That hero would be placed in e.g. the default layout of the article node type. It would never make sense to expose neither the text nor the image for the "Christmas hero" in Views.

I've certainly been asked to do things like a slideshow of featured articles using their hero image, article title, field from a related entity, etc. I definitely want to be able to get at that sort of thing via views.

🇳🇿New Zealand John Pitcairn

My concern is I don't think we should be asking content-editors to 'pick the field you want to populate this prop from'. I think site-builders should make that decision ahead of time.

Strongly agree with this.

Not sure about needing to define multiple components if there is a choice of prop fields. I can see that becoming tedious.

I can see a need for allowing the site builder to configure a component that would present, say, a select menu to the editor for choosing between a couple of predefined source fields to populate a prop. The site builder would be responsible for providing good option labels and help text to support the editor.

Not MVP, maybe something for contrib if the extension point is there.

🇳🇿New Zealand John Pitcairn

While using a value of "0" in a list (text) field is perhaps ill-advised, it should be possible, and it should definitely be possible to use a value of 0 in a list (integer) field. But that produces the same error.

I guess something is testing whether there's a value using empty() when really it should be calling isset() or a strict equals.

Re-titling to reflect the more general problem. Fortunately I found this before pushing the new field to production.

🇳🇿New Zealand John Pitcairn

EVA is probably the most common way to accomplish what you want, if entity references or a block-based solution using the URL are not sufficient. I've been using it for years.

32 open bugs on a module with 25000 sites recorded using it is a low bug count. Only a couple of those are rated major, none critical.

🇳🇿New Zealand John Pitcairn

The above would be great for straightforward views access to component fields, since they're just regular fields on the entity. That's been a real content model pain point with inline blocks in layouts (ie where you can't just use a simple field).

It would presumably make content moderation "just work" for multi-field components too.

How might we handle a "make this reusable" action, when the editor wants to re use a field-based component they added to this layout on another layout somewhere? That's a very common requirement.

🇳🇿New Zealand John Pitcairn

@rgpublic: I totally agree.

We often find ourselves in the situation of wanting to update entity field values in a deploy hook. Changed assumptions, bad data entry, whatever.

Or admins need a way to generate and save a reference to an external api entity that failed to create (flaky api, whatever).

In both cases we don't want to mess with the changed timestamp (that just confuses editors), and we are not "syncing" a migration.

🇳🇿New Zealand John Pitcairn

Using patch #80 against 10.2.3 allows local task tabs to highlight as active if there is any added url query.

I have not tested what happens if the local task target url itself contains a query. In that case the behaviour should probably be to match the local task specific query parameter(s) and only highlight as active if the task url query parameters match, ignoring added query parameters. @casey is this what #84 is attempting?

🇳🇿New Zealand John Pitcairn

Yes try it.

🇳🇿New Zealand John Pitcairn

Try uninstalling the Internal Page Cache module.

If that works, install the Dynamic Page Cache module.

🇳🇿New Zealand John Pitcairn

^^ This. Being able to simply declare what entity list tags you depend on is a lot more transparent than having to figure out which ugly procedural hooks you need to implement (update and insert, what?), and what you have to do in those to then invalidate the entire plugin cache.

The DX improvement is more than minor IMO.

🇳🇿New Zealand John Pitcairn

In this case the issue is simply module weight. My custom module is sorted alphabetically before "charts", so it just needed a weight decrease in libraries.yml.

Thanks @andileco for taking the time to look at this.

🇳🇿New Zealand John Pitcairn

Re #14 and #15: This is not specific to multi-column fields. I can reproduce it with a single entityreference field in a view of commerce subscriptions.

I can set aggregation for the field to COUNT, and apply.

But thereafter I cannot remove or change the aggregation for the field. Clicking apply will produce the ajax error.

Un-postponing.

🇳🇿New Zealand John Pitcairn

Note if you use a dedicated media type for SVGs (ie"logo" in my case), you can set the image formatter to output "original image" for all view modes, and that will display just fine in the media library and widget.

🇳🇿New Zealand John Pitcairn

That's what this issue is about. The regression is that there used to be a fully responsive jpg fallback, and now there is not.

The patch does not restore that functionality.

🇳🇿New Zealand John Pitcairn

@ChristianDeLoach, could you please check the following:

The img tag should also contain a srcset attribute listing the responsive jpg versions.

If the img tag only contains a src attribute, then there is no responsive jpg support for browsers that do not support webp. There is still only a single jpg fallback.

🇳🇿New Zealand John Pitcairn

Maintainers are not responding to contact messages, and are not on Drupal slack. I think we can consider this module abandoned.

🇳🇿New Zealand John Pitcairn

I can also reproduce this warning when opening the media library from a paragraph edit modal.

🇳🇿New Zealand John Pitcairn

I'm running into this error when running the views_post_update_remove_default_argument_skip_url db update during a 9.5.11-to-10.2.2 upgrade. While the null check may indeed be masking a problem with the view config dependencies, the patch at #28 does at least allow the update to proceed.

🇳🇿New Zealand John Pitcairn

#261 and #262 will not apply to 10.2.x, needs a reroll.

🇳🇿New Zealand John Pitcairn

::findField() returns an element, pass its value to xpath.

🇳🇿New Zealand John Pitcairn

We are no longer guaranteed a string return when finding the message field.

🇳🇿New Zealand John Pitcairn

OK, not sure how to get this MR to test. Patch created from #16.

🇳🇿New Zealand John Pitcairn

Re #5: I think the DrupalCheck functional js errors may relate to 📌 Fix PHPStan L2 error in traits Needs work . The problems DrupalCheck identifies in the .module file are nitpicks.

Unassigning and setting to needs review, let's test the MR.

🇳🇿New Zealand John Pitcairn

Is this about entity reference field autocomplete?

If so, what's wrong with the established technique of using a view to provide the entity suggestions. That does allow you to specify the entity fields to search on.

🇳🇿New Zealand John Pitcairn

If there is help text, it should not say "removed".

🇳🇿New Zealand John Pitcairn

IMO any other library-linked paragraph should remain so, including any other copies of the paragraph in the same parent entity. Only this single local paragraph should be unlinked.

Anything else would be a UX WTF, and hard to explain.

🇳🇿New Zealand John Pitcairn

Composer install just puts the required directory structure, drupal files and php dependencies in place. It does not install the drupal database.

You need to do that yourself, by visiting /install.php, or by running drush site:install.

You need to have created the database, db user and db password before doing so.

🇳🇿New Zealand John Pitcairn

Maybe they are confused about how Drupal permissions operate. I've seen this sort of behaviour from "security consultants" before. If they are running penetration scans, they should be doing that with low or no privileges (anonymous or normal registered user), not as a trusted full-admin user. 

🇳🇿New Zealand John Pitcairn

Probably just running out of php memory on the other end of the ajax request. Forget dev console for this, it's only really useful for js errors, check the Drupal logs for more accurate error messages: /admin/reports/dblog

🇳🇿New Zealand John Pitcairn

Ha. I mean the default in the formatter, where defaults are currently hard coded. Probably has to be add an #after_build callback to the entire form.

🇳🇿New Zealand John Pitcairn

Is any work happening @thomas.frobieter? It's assigned to you.

Is there a current workaround if I want the default for all elements and wrappers to be "None"?

🇳🇿New Zealand John Pitcairn

By the way, all the 403 should be switch to 404 for security obfuscation.

Only if Drupal does that in .htaccess for Apache as well. It's a misuse of the response code.

Sites are free to modify the defaults for their own purposes, if obfuscation is a requirement.

🇳🇿New Zealand John Pitcairn

Fair enough. No on second thought it's a problem that will go away eventually by itself with these fixes in.

🇳🇿New Zealand John Pitcairn

Safari 17 is quite a high bar given Apple ties it to specific OS updates (it's the new IE). Any interest in a fallback technique with broader browser support?

🇳🇿New Zealand John Pitcairn

Just bitten by this with a minimal install. Not the first time either from memory. MR3639 does appear to fix the problem.

Is this still a 10.x candidate or does it need to be 11.x now?

🇳🇿New Zealand John Pitcairn

destination => $return_url->toString()

🇳🇿New Zealand John Pitcairn

Is it possible to mark the composer package for 2.x as unsupported? Anyone running composer require drupal/inline_entity_form is still going to get ^2.0@RC.

🇳🇿New Zealand John Pitcairn

Doesn't the standard entityreference formatter already include some protection against recursive rendering?

🇳🇿New Zealand John Pitcairn

My client's expected workflow looks like this:

states:
    initial:
      label: Unpublished draft
      published: false
      default_revision: true
    published:
      label: Published
      published: true
      default_revision: true
    draft:
      label: Pending draft
      published: false
      default_revision: false
    unpublished:
      label: Unpublished
      published: false
      default_revision: true
🇳🇿New Zealand John Pitcairn

This was biting me today as part of a broader attempt to make moderation behave the way a client expects.

My current solution is to swap out NodeModerationHandler and override ::onPresave() to avoid changing the default revision unless we are publishing. The handler is marked @internal however so that's not ideal.

🇳🇿New Zealand John Pitcairn

Anything that can't be googled should not be an official support forum. Dries' recent Lille keynote was very big on pushing the open web - Slack isn't that.

I doubt all the devs on Drupal Slack will be all that thrilled with a bunch of non-coder support requests clogging things up either.

🇳🇿New Zealand John Pitcairn

@Jaypan: 

The main problem I see with this is that when you google an issue, you find forum links. Slack conversations do not show. So there is no record of support.

Exactly. Slack channels are great for fast comms between developers, but if we are going to deprecate support in favour of something the DA doesn't need to maintain (if that's the rationale), then at least (say) Stack Overflow can be googled.

I doubt all the devs on Drupal Slack will be all that thrilled with a bunch of non-coder support requests clogging things up either.

🇳🇿New Zealand John Pitcairn

Duplicate issue, your form is misconfigured for stock support. Please see this comment Support for Commerce Stock Enforcement Fixed and the rest of that thread.

🇳🇿New Zealand John Pitcairn

I'm closing this. The solution I recommend is to build the comparison view you need, and add a rendered entity field to display the add-to-cart form for each row.

The requirements for the comparison view will be different for every site depending on the product variation fields and attributes, so I don't feel it is a good candidate for a generic solution. Existing tools should be sufficient to build what you need.

🇳🇿New Zealand John Pitcairn

Re #66: yes, but this issue does not just affect text formats/editors.

It also affects things like views rewrites, where you might want to rewrite and add a style attribute that is benign - passing a css property value through to a stylesheet for example. No text editor is involved there.

🇳🇿New Zealand John Pitcairn

Yup but any fix will be against 11.x then backported, right?

I've definitely noticed Claro blockquote styling leaks into the editor. Annoying.

🇳🇿New Zealand John Pitcairn

I also agree with #31.

Content editors who should only interact with blocks via layout builder should not see the reusable blocks overview page or be able to create standalone reusable blocks from there.

🇳🇿New Zealand John Pitcairn

Yow. I can reproduce #98 as user 1 with full admin privileges.

JS console error:
ResponseText: {"message":"Non-reusable blocks must set an access dependency for access control."}

Watchdog:

Path: /media-library?destination=/node/4/layout&_wrapper_format=drupal_ajax&ajax_form=1&media_library_opener_id=media_library.opener.field_widget&media_library_allowed_types%5Bimage%5D=image&media_library_selected_type=image&media_library_remaining=1&media_library_opener_parameters%5Bfield_widget_id%5D=field_image%3A-settings-block_form&media_library_opener_parameters%5Bentity_type_id%5D=block_content&media_library_opener_parameters%5Bbundle%5D=image&media_library_opener_parameters%5Bfield_name%5D=field_image&media_library_opener_parameters%5Bentity_id%5D=2&media_library_opener_parameters%5Brevision_id%5D=2&hash=RZpw_Ue-CORgfs34fAruBGXBLZUhx_qIoGwlq0UPRZU&views_display_id=widget&_wrapper_format=drupal_ajax.

Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException: Non-reusable blocks must set an access dependency for access control. in Drupal\Core\Routing\AccessAwareRouter->checkAccess() (line 118 of /var/www/html/public/core/lib/Drupal/Core/Routing/AccessAwareRouter.php).
🇳🇿New Zealand John Pitcairn

Or even add a checkbox to each individual theme's settings. Maybe that's a followup ;-)

🇳🇿New Zealand John Pitcairn

I don't think Video module can record video directly into the form, can it? It's a file upload.

🇳🇿New Zealand John Pitcairn

Well, did you know that more than 80% of Deaf people in the world don't understand text?

You mean can't read? How are they navigating the website in that case?

🇳🇿New Zealand John Pitcairn

It seems like a use case that would not be uncommon. I'm surprised there isn't anything.

At a minimum you'd need to swap out the default add-to-cart form class and write your own custom form class that (probably) extends the default form. If you also need to use the default form on some some products, you'd need a field formatter so you can choose which form to use on the variations field rather than blindly swapping it.

Have a look at these first:

Commerce Add To Cart Matrix - seeking new maintainers, but looks like it does almost exactly what you want.

Commerce Variation Add To Cart . That operates at the product level and I think it builds a single form. But last time I looked at that it was building the form entirely in a twig template which is not a good way of doing this. It may have changed.

🇳🇿New Zealand John Pitcairn

No sorry. This module adds a separate form to each individual product variation. That's all it does.

It sounds like you need a single customised add to cart form on the product.

🇳🇿New Zealand John Pitcairn

I cannot reproduce this in Drupal 10.1.

🇳🇿New Zealand John Pitcairn

It's designed to accommodate complex situations where you really do want to be able to do complex things with attributes and variations. Things you couldn't do in Ubercart.

You know you can just duplicate a variation and change the fields that are different, right?

🇳🇿New Zealand John Pitcairn

The tab should be "Latest revision" by default.

I have users really struggling to understand Drupal revisions, especially with content moderation in the mix. The problem is "current revision" does not necessarily mean "published". It could be:

  • The latest revision of an entity that has never been published. This is also the default revision.
  • The published revision, with no forward drafts. This is also the default revision.
  • The published revision, with some forward drafts on top of it. The latest draft is the default revision.
  • A previously-published revision, with no forward drafts. This is also the default revision.
  • The latest forward draft of a previously-published revision. This is also the default revision. Current no longer corresponds to a revision that is or was published - WTF?

Given the above, I don't think "current revision" is a useful term in a list of revisions. It's only relevant internally. Editors want to be able to easily identify:

  1. The published revision.
  2. If there is no published revision, the last-published revision.
  3. The editable revision.
  4. Whether the entity as a whole is published (there may be a long list of forward drafts).

Then the revision action labels and confirmation form text needs to be different, depending on what the actual outcome will be:

  • Will the revision become published?
  • Will the revision become the new editable revision?
  • Are there any forward drafts that will become orphaned?

We have workflow state transitions and state transition labels. Can we make use of those?

🇳🇿New Zealand John Pitcairn

You might get more developer attention if you join drupal slack and post in the accessibility channel there.

🇳🇿New Zealand John Pitcairn

Reverting incremental changes is not easy, especially when the taken action isn't well understood.

Related to that: 🐛 Content Not reverting in Layout builder section Active

🇳🇿New Zealand John Pitcairn

As a site editor, I would always expect reverting to discard any layout changes. I want the content and layout in sync on revert.

I cannot imagine a situation where not doing so makes any sense, and I imagine legal might take a fairly dim view of anything otherwise occurring.

🇳🇿New Zealand John Pitcairn

I think anyone using layout builder lock and customising the render element in css or js may want to have eyes on this. It's possible somebody may be relying on the absence of the .layout-builder-block class.

🇳🇿New Zealand John Pitcairn

Ah we do need to remove the .js-layout-builder-block class. That will allow themes to target .layout-builder-block, but should prevent any js actions (which should be targeting the js- class).

🇳🇿New Zealand John Pitcairn

Like so. We can remove the for() loop entirely.

I also used a less specific selector than .layout-builder-lock.layout-builder-lock to be a bit friendlier to themers.

🇳🇿New Zealand John Pitcairn

It also needs to not remove the class in prerender.

🇳🇿New Zealand John Pitcairn

What is the "product" that has the ID?

🇳🇿New Zealand John Pitcairn

Have you added the cweagans/composer-patches package?

composer require cweagans/composer-patches

You also need:

"extra": {
    "enable-patching": true,
    "patches": {
         ...
    },
🇳🇿New Zealand John Pitcairn

Can you get to /admin/reports/dblog ?

🇳🇿New Zealand John Pitcairn

If you're concerned about GDPR, and it works for you, push for both a stable release and Drupal security advisory plociy coverage.

🇳🇿New Zealand John Pitcairn

Making this change means any other theme that overrides the layout builder lock css rules for padding or pointer will probably need to update their rules to be more specific as well - though they could just override the library entirely to avoid that.

I'm using a custom front end theme. We modify the layout builder css a lot to make it match the page view. This is a simple example of what we will do for a locked section:

🇳🇿New Zealand John Pitcairn

Is there any reason you can't add a views field of "rendered entity" in your view that compares variations, and set that to use a view mode that only displays the add to cart form?

You can't use the rewrite option on that field (it will strip form tags), but otherwise it should work.

🇳🇿New Zealand John Pitcairn

OK and when you switch off legacy mode, existing legacy block instances will remain in the layout, but you wont be able to add new legacy blocks. Gotcha.

Production build 0.69.0 2024