Account created on 18 September 2017, about 7 years ago
#

Merge Requests

More

Recent comments

🇨🇦Canada nickdjm

A readme has been added with some basic usage/documentation information.

🇨🇦Canada nickdjm

The fix works in both D10 and D11 now.

🇨🇦Canada nickdjm

Tested in Drupal 10.3.8 and get the following:

PHP Fatal error: Type of Drupal\accessible_menu\Form\SettingsForm::$typedConfigManager must not be defined (as in class Drupal\Core\Form\ConfigFormBase) in /var/www/html/web/modules/contrib/accessible_menu/src/Form/SettingsForm.php on line 17

We'll need to figure out a way to make this work in both 10 and 11.

🇨🇦Canada nickdjm

I can't seem to reproduce the error you're seeing following the steps provided.

I made a fresh install, enabled the module, enabled the workflow on content blocks (which I will comment on shortly), added it to the block layout and created an article. All works as expected. No errors seen.

Is there something I'm missing?

Also, it is worth noting that enabling the workflow on the block itself will basically do nothing as far as moderation_state_condition is concerned. This module currently only supports nodes. If you enable your workflow on the article and then place a block to conditionally show on a specific workflow state you should see the results you (hopefully) want. When I have some time I'll add other entity support, but until then it's only nodes.

🇨🇦Canada nickdjm

This feature can be found in v3 of this module (which is still in alpha). It's included in a sub-module called Migrate Default Content Export.

🇨🇦Canada nickdjm

The json format will work as well, but that's more so because both json and yaml are valid yaml.

🇨🇦Canada nickdjm

Just to update this issue.

bodyValue, bodyFormat, and other field properties like that are going to be removed in v3. We've been running into a handful of issues with it, and it's much cleaner to just have the content in proper objects.

The only accepted format moving forward will be:

body:
  value:
  format:
🇨🇦Canada nickdjm

Perhaps I'm misunderstanding this issue, but when I attempt to create an entity reference field that does not specify a bundle I get a form validation error. It looks like this might have been an issue with core and you cannot have an entity reference field that has no bundles now.

Seeing as how this hasn't been looked at for ~3 years and we haven't run into this issue at all I'm going to close for now. We can open a new issue if it comes up again.

🇨🇦Canada nickdjm

If we implement this feature it will be for the 3.x branch. Updating issue to reflect.

🇨🇦Canada nickdjm

Just popping into this issue to say I have a feature branch with a drush command (migrate_default_content:export-migrations [mdcem]) that allows you to export the generated migrations.

It also provides a new setting on the settings form to control where those migrations get exported to.

Once I get a little more testing done and possibly add some options for only exporting specific migrations then it'll be merged into the 3.x branch.

All new drush commands are being written for Drush 12 with (currently) no plan to backport them to older versions, so to get this feature you'll need to be on Drupal 10.2.x with Drush 12.

🇨🇦Canada nickdjm

Reviving this issue since it's very much still a bug. Looked over the change, tried it out. Looks good.

I'll get this merged and push to both 8.x-2.x and 3.x

🇨🇦Canada nickdjm

This feature has been rolled into 3.0.0-alpha0. Please use that version of the module for the ability to export content.

We will keep track of any other issues related to this (since I'm sure there will be some) in their own issues.

As for this issue, I consider it fixed.

🇨🇦Canada nickdjm

An update on this: me rerolling the patch and fixing the drush commands didn't work. There's some other weirdness going on with layout builder I think.

That being said, I have a branch off the main module where I am implementing this as it's own module.

On my testing site I have it working. It exports content in all languages and puts them into yaml files as expected. I still need to test it out on other sites (and get layout builder support in), but it looks promising.

I have added filters so you can include/exclude entities, bundles, and ids.

🇨🇦Canada nickdjm

Updating the issue to point to the version of MDC that the MR applies to.

🇨🇦Canada nickdjm

Ok MR has been created and everything is working about in D10.2.2.

The next steps will be to refactor this into something we can split out properly into a submodule: migrate_default_content_export

First will be to get basic entities working (nodes, blocks, terms, etc.) with no layout builder functionality. Once this is done, we'll look at the layout builder stuff.

🇨🇦Canada nickdjm

Just to give an update to this issue:

The current patch in #11 does not work. I have a fix that I will be posting shortly in a MR with the updated drush command syntax.

As for this issue as a whole, we will be splitting this out into a sub module. This functionality is super useful and we don't want it to live as a "forever patch", however the code written in the patch needs a lot of work. At a bare minimum it does not meet the Drupal coding standards, nor is it documented sufficiently.

Once we have a MR open for this, it will be kept alive while we work on a new branch with cleaned up code. The first goal will be to allow exporting of what MDC can already handle (basically removing the layout builder functionality for now), then we will be reducing the number of drush commands to a single command that allows users to pass options to it for what entities/bundles they want. Once this is functional we will implement the Layout Builder functionality.

I'd also like to give a big thanks to everyone who has contributed to this issue so far. We use this patch constantly. It's time to see it implemented officially.

🇨🇦Canada nickdjm

In this case I think extending the class, while understand the risks of doing so, is the right way to go.

I have made the other suggested changes to the 3.x branch, so this is now ready for review again.

🇨🇦Canada nickdjm

I mean sure, I can copy paste the class... But there are all sorts of traps there are there not? Creating a brand new copy of FieldItemList just for the sake of not using the original class would open up a whole new can of worms if anything tries to check if my list is a FieldItemList. Kind of defeats the point of object oriented coding.

If it's what I need to do I can do it, but that feels really wrong.

🇨🇦Canada nickdjm

For group_computed_field.install

I have removed the install hook entirely and removed the definition removal from the uninstall hook, however I left in the functionality that removes "related_groups" from entity_view_display configs. In my testing, uninstalling the module does not automatically remove the field from the configs that used it.

🇨🇦Canada nickdjm

For src/Plugin/Field/RelatedGroupFieldItemList.php:

Are you referring to me extending FieldItemList? I was basing this off of core modules (like content_moderation) where the ModerationStateFieldItemList extends FieldItemList.

Would I need to make my own base class that extends FieldItemList and then use that in the plugin?

🇨🇦Canada nickdjm

I also created Group Computed Field ( https://www.drupal.org/project/group_computed_field )

My co-maintainer already opted it in to security coverage, but I wrote pretty much all the code for it. I wasn't sure if it would count since it was already opted in.

That all being said, I don't know if it has enough PHP in it either- it's a pretty small module.

If it would be a better candidate I can make a new application or alter this one if that works.

🇨🇦Canada nickdjm

I added migrate_source_yaml back to dependencies because it creates a pour user experience if migrations that we have examples for don't work out-of-the-box.

🇨🇦Canada nickdjm

Reviewing now. I am going to spilt out the dependency changes into their own issue since that's big enough to warrant it.

🇨🇦Canada nickdjm

NickDJM made their first commit to this issue’s fork.

🇨🇦Canada nickdjm

Just to have this documented: I am able to get link attributes working with no code changes required when I use the following structure:

-
  title: Twitter
  menu_name: social-links
  uid: 1
  weight: -10
  link:
    uri: https://twitter.com/myhandle
    options:
      attributes:
        class: icon-twitter

We'll add it to the project examples since link attributes is a pretty common module to use.

🇨🇦Canada nickdjm

Rerolled #57 for latest version.

🇨🇦Canada nickdjm

I've run Upgrade Status on this module, and this patch is all that's needed for D10 support. Everything else is compatible.

🇨🇦Canada nickdjm

This is in dev. Will be in the next release.

🇨🇦Canada nickdjm

This has been resolved in the latest release.

🇨🇦Canada nickdjm

In our case we are using hrs to separate days. I only suggested span and div because you can add specific classes to those elements (in case someone is using Bootstrap or TailwindCSS for their theming).

🇨🇦Canada nickdjm

Further testing of both my patch and the previous patch has revealed that properly identifying the clicked button works in some scenarios, but not others.

In the case of my patch, it actually was breaking layout builder's submit buttons because they all have the same name- so disregard my patch.

🇨🇦Canada nickdjm

I have updated the patch in 57 to fix an issue causing views apply/cancel/remove buttons to not submit properly.

The AJAX call _was_ adding a value to the "op" input, so the logic I had in 57 was flawed. We should only assume the input was clicked if the value matches exactly or if the value is exactly empty. Anything other than those 2 outcomes means we can't assume that button was clicked.

🇨🇦Canada nickdjm

As a note: I know this needs some work, just wanted to get a patch posted to build upon.

🇨🇦Canada nickdjm

So I've been debugging this in 9.5.10 and none of the patches above seem to identify the triggering element properly.

I tracked the issue down to the buttonWasClicked() method in FormBuilder. It specifically checks to see if the name and value of a button is found in the getUserInput() array, but buttons do not submit values. Their value is used as the text to display in the button, if if they are added to the getUserInput() array the value will be an empty string and never match to criteria for marking the button as clicked.

I also noticed that the key for any button on the form will only show up in the getUserInput() array if it was the button used to actually trigger the form submission.

I wrote a patch that adds another check to buttonWasClicked() that checks to see if a button's name is a key in the $input array, and if it is, it flags the button as the one that was clicked and results in the proper triggering element being flagged.

🇨🇦Canada nickdjm

Tried the patch in #18 and it did the trick.

I was having an issue with a view using an ajax pager. With #18's patch the patterns render as expected when hopping between pages.

🇨🇦Canada nickdjm

I tried path #8 and it does works on a fresh install of the module. The schema changes are not picked up on a site where the module is already installed, so we'll need an update hook to resolve this properly.

🇨🇦Canada nickdjm

Also as a note: The changes in the MR have resolved the issue on one of my sites (9.3.9) I'll be applying it to a couple of other sites as well to see if it works on them.

🇨🇦Canada nickdjm

I'm not entirely sure why, but when you look at https://git.drupalcode.org/project/drupal/-/merge_requests/782.patch there are 4 patches that attempt to apply themselves- 1 and 2 are valid, while 3 and 4 are identical to the previous two- just with different hashes. This causes the patch to fail since its essentially just trying to patch everything twice.

I've created a de-duped patch in case anyone wants to apply it that way.

This is valid as of commit 44fc2dee21615c2569eed8b512e9877aee827ae6.

Production build 0.71.5 2024