NH, USA
Account created on 25 January 2007, over 18 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States damienmckenna NH, USA

Other modules that interact with Layout Builder seem to have the same bug, e.g. 🐛 Settings form broken: Field layout_builder__layout is unknown. Active .

🇺🇸United States damienmckenna NH, USA

Should the views schema definition go in the facets_exposed_filters module?

🇺🇸United States damienmckenna NH, USA

WIP test coverage for the facets_demo module.

🇺🇸United States damienmckenna NH, USA

FYI it might be worth adding test coverage for the facets_demo module as a base to work from for this.

🇺🇸United States damienmckenna NH, USA

Yes, this is the same as 🐛 Views filter schema Active .

🇺🇸United States damienmckenna NH, USA

I copied the two tests and adjusted their namespaces.

When I ran NodeTitleTest locally the test failed because the output doesn't have a H1 tag, so I think we should remove that one.

🇺🇸United States damienmckenna NH, USA

Doing a quick copy & adjust.

🇺🇸United States damienmckenna NH, USA

It turns out that 11.2.x only has two tests:
* core/themes/claro/tests/src/Functional/NodeTitleTest.php
* core/themes/claro/tests/src/Functional/MenuLinkDefaultFormTest.php

I'll review them to see whether each test is also covered by a Gin test.

🇺🇸United States damienmckenna NH, USA

I created a sub issue for the tests: 📌 Recreate/copy Claro tests to Gin 6.x Active

🇺🇸United States damienmckenna NH, USA

For the test coverage, would it be better to just copy all of Claro's tests and tidy them up, or just recreate them as needed?

🇺🇸United States damienmckenna NH, USA

I provided a MR that adds a note about vocabulary matching.

🇺🇸United States damienmckenna NH, USA

Lauri told me that keeping the field would still be better as it would give the option to create a custom, simpler widget for more complicated SEO settings in the future.

I originally planned to build a new widget for making it possible to customize which meta tags were shown on the edit form, but realized it was pointless when you could just add regular fields and then use tokens to output the tags as necessary.

I don't believe there's any benefit to the Metatag field as it is currently implemented, it feels like it was added as a misunderstanding of how & why it works; my recommendation would be to remove it and just use tokens in the global configurations.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

Pameela: Would you mind linking to an issue that describes the plan to replace content moderation with workspaces? Thank you.

🇺🇸United States damienmckenna NH, USA

I was actually asking about the facet blocks.

🇺🇸United States damienmckenna NH, USA

Should this be a feature request?

🇺🇸United States damienmckenna NH, USA

The idea with Metatag is to define defaults for each entity type at the global level using tokens to pull in values from the entities. Take a look at Configuration → Search and metadata → Metatag, and customize the configuration for each entity type. You can also override the entity type configuration on a per bundle basis via the "Add default meta tags" page, so that e.g. different content types could have slightly different values based upon the fields available. In theory this should cover most scenarios, especially if you have a few fields on the entity type/bundle for a summary/description and a thumbnail image. IMHO most sites don't need the Metatag field on content types, they should leverage the defaults, fields and tokens.

Also, if you add the Token OR module you can have either-or logic within each token, useful for setting a failover default if a field isn't filled in.

🇺🇸United States damienmckenna NH, USA

Does metatag [module] need the [full metatag] field to be present to pull from the description and image base fields for metadata?

No, Metatag can just use tokens in a global configuration to generate meta tags for content entities based upon those few fields, so removing the Metatag field for now would probably be the best bet.

🇺🇸United States damienmckenna NH, USA

Thank you for putting together a merge request for this, it looks good.

🇺🇸United States damienmckenna NH, USA

XB shouldn't be doing weight things like that via hook_entity_base_field_info, it should just add the field via Field API.

🇺🇸United States damienmckenna NH, USA

I wasn't able to reproduce this. I wonder if it was because of 🐛 Invalid NULL value in json_widget.js Active ?

🇺🇸United States damienmckenna NH, USA

This needs an update script to fix existing fields that have the wrong size.

🇺🇸United States damienmckenna NH, USA

You are completely correct, these need to be supported. Let's add test coverage too.

🇺🇸United States damienmckenna NH, USA

Committed. Thanks everyone.

🇺🇸United States damienmckenna NH, USA

damienmckenna changed the visibility of the branch 8.x-1.x to hidden.

🇺🇸United States damienmckenna NH, USA

The current merge request doesn't do anything useful, so I've closed it.

🇺🇸United States damienmckenna NH, USA

Committed. Thank you.

🇺🇸United States damienmckenna NH, USA

Good catch, thank you.

🇺🇸United States damienmckenna NH, USA

Sure, that's definitely doable. I would strongly recommend changing the multi-value separator to something other than a comma, though.

🇺🇸United States damienmckenna NH, USA

I created a merge request to let the tests run so we can see how the change behaves against different core branches.

🇺🇸United States damienmckenna NH, USA

I wonder would removing the field from the advanced section resolve the problem instead? That's an option on the field widget.

🇺🇸United States damienmckenna NH, USA

This resolves the problem with 10.5.0.

I think we should create a new minor branch (1.1.x) for this fix, then update 1.0.x to limit its compatibility to <10.5 and <11.2, then the new 1.1.x branch requires 10.5 or 11.2.

🇺🇸United States damienmckenna NH, USA

FYI while this probably would have fallen under PSA-2023-07-12 , this should have been reported privately as a security issue and then discussed with the Drupal Security Team. Please try to keep this in mind if similar issues show up again. Thank you.

🇺🇸United States damienmckenna NH, USA

On a website I'm using this with I'm seeing some unexpected behavior where terms are being placed under incorrect parent items. For example, I have one term which does not have a parent in the database, but it is placed under a term which isn't even in the vocabulary that the first term comes from.

🇺🇸United States damienmckenna NH, USA

On a local barebones 11.1.x install with the latest Facets 3.0.x I could not reproduce the problem that this resolves.

We might need to dig into it a little more and add test coverage to confirm exactly what behavior we're after, because I wonder how much of it is a data problem?

🇺🇸United States damienmckenna NH, USA

Would it be worth a separate issue to simplify the implementation logic so the extra $databases[..]['dependencies'] logic could be automatic based upon the class hierarchy rather than needing to be explicitly defined?

🇺🇸United States damienmckenna NH, USA

This works great, once you set the processors to the correct order, e.g.:
* Show parents
* URL handler
* Transform entity ID to label
* Build hierarchy tree

Marking RTBC to get the maintainer to take a look.

🇺🇸United States damienmckenna NH, USA

This works great - there's no reason the library should be loaded on every page.

🇺🇸United States damienmckenna NH, USA

IMHO this is a bug report, and when enabled is the expected behavior.

I've been pulling my hair out over this, trying to work out why the term hierarchy is not shown, only for it to be a problem with the module.

Thank you for this fix!

🇺🇸United States damienmckenna NH, USA

FYI I'm getting lots of this error after trying the change:

Deprecated function: Invalid characters passed for attempted conversion, these have been ignored in Drupal\Component\Utility\Number::intToAlphadecimal() (line 80 of core/lib/Drupal/Component/Utility/Number.php).

🇺🇸United States damienmckenna NH, USA

FYI the separate provider module has been deprecated in favor of this issue, though the current MR doesn't apply against the 3.x branch.

🇺🇸United States damienmckenna NH, USA

I tried modifying the playwright.config.js file as follows:

  reporter: [
    'html',
    {
      host: 'damiend10.ddev.site'
    },
  ],

(where "damiend10.ddev.site" is my hostname)

.. but it doesn't make any difference, it still shows the error.

🇺🇸United States damienmckenna NH, USA

Please check to see if you still have the metatag.tokens.inc file in your metatag directory - it was deleted in 2.1.1, but it might still be present.

🇺🇸United States damienmckenna NH, USA

damienmckenna changed the visibility of the branch 3.0.x to hidden.

🇺🇸United States damienmckenna NH, USA

Let's make this a minor UI improvement by making it not required but improving the help text to better indicate when it should be added.

🇺🇸United States damienmckenna NH, USA

Thank you for taking up that responsibility!

I'm not personally interested in it, but maybe others might be?

🇺🇸United States damienmckenna NH, USA

Thank you for the patch you've provided so far.

As a reminder, please set the "Assigned" field to "Unassigned" after you're finished your work, that way others can know that you're finished and that it's ready to review.

🇺🇸United States damienmckenna NH, USA

Per 💬 Schema.org: Service Active we need to make sure these attributes are included too:

* serviceType
* areaServed
* description

🇺🇸United States damienmckenna NH, USA

Thank for reporting this.

This would be a feature request to extend the existing structure with these new attributes.

🇺🇸United States damienmckenna NH, USA

I'm sorry but with D7 in EOL we won't be digging into this anymore.

🇺🇸United States damienmckenna NH, USA

Note: I missed that there is an update script that adds an initial attribute for each custom tag already in the system, apologies for my oversight and thank you for taking care of that already.

The other issues still remain though.

🇺🇸United States damienmckenna NH, USA

FYI I think we should postpone further discussion here until 📌 [Policy] Define explicit Drupal 10 end of life date of 2026 December regardless of Drupal 12 release Active is decided upon, as it might make it moot.

🇺🇸United States damienmckenna NH, USA

While this is still being worked on, I created a separate MR that improves the JS output of the checkbox widget: Add missing classes to checkbox widget so it renders correctly Active

🇺🇸United States damienmckenna NH, USA

In my local testing this is working better than the current 3.0.0 branch - checkboxes float correctly beside the labels, labels don't wrap underneath the checkbox, etc.

🇺🇸United States damienmckenna NH, USA

It might be worth reverting this change to avoid breaking backwards compatibility.

Production build 0.71.5 2024