šŸ‡ŗšŸ‡øUnited States @codechefmarc

Account created on 17 July 2014, over 10 years ago
#

Merge Requests

More

Recent comments

šŸ‡ŗšŸ‡øUnited States codechefmarc

I just added an MR to address this issue. It uses the core administer menu permission, not a custom one, unless we want to add a specific permission for that.

šŸ‡ŗšŸ‡øUnited States codechefmarc

codechefmarc ā†’ made their first commit to this issueā€™s fork.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Tested on version 2 and on version 4 and works great! Thank you for the quick turnaround, I was going to create the MR today but you beat me to it. :)

I ran the tests and the only one that didn't pass was the BlockClassControllerTest.php due to this issue ( https://www.drupal.org/project/block_class/issues/3412155 šŸ“Œ Deprecated system_get_module_admin_tasks in drupal:10.2.0 and is removed from drupal:11.0.0 Needs review ) which is known and isn't a blocker for this MR because that test only covers the help page.

šŸ‡ŗšŸ‡øUnited States codechefmarc

codechefmarc ā†’ made their first commit to this issueā€™s fork.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Ahh, it seems there was also another issue that takes care of this: https://www.drupal.org/project/entity_redirect/issues/3462802 šŸ› Create new node call hook_entity_insert and hook_entity_update both. Active

When will the dev version be pushed to a new version? Thank you!

šŸ‡ŗšŸ‡øUnited States codechefmarc

Fixed in beta2

šŸ‡ŗšŸ‡øUnited States codechefmarc

This was fixed in beta2.

šŸ‡ŗšŸ‡øUnited States codechefmarc

codechefmarc ā†’ made their first commit to this issueā€™s fork.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Added earlier this week.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Fixed in 1.1.0.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Added in 1.1.0.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Fixed in 1.1.0.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Thank you, @priyanka_chauhan31 for the patch. I've applied to dev version and will be part of the latest release coming out soon.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Hi there, I've updated the code as per the requested changes. I've also removed the main branch as requested. Please re-review. Thank you!

Here is the latest update: https://git.drupalcode.org/project/reading_rating/-/tree/1.0.x?ref_type=...

šŸ‡ŗšŸ‡øUnited States codechefmarc

Hi, so sorry things have been very busy lately. I do finally have some time to work on this today and will update soon. Thanks for keeping tabs on it.

šŸ‡ŗšŸ‡øUnited States codechefmarc

I was doing some research into adding the defaultHeadings and came across that link you posted @ahmad-khader and was curious if adding a default 'rows' => 1, is a good idea to core or not. In doing accessibility testing, some testing tools will report a warning that the table is a layout table, and not a data table, reducing accessible tables.

I had thought we could include this in the standard profile (soon to be recipe), but I played with the config and could not get it to work:

In editor.editor.basic_html.yml config, I see the following structure:

settings:
  plugins:
    ckeditor5_heading:

And so I thought I could follow that by adding the defaultHeadings to it. I tried many different iterations but could not get it to recognize it. So:

1. Is config the way to go with this? (I assume so as that makes the most sense to me) and if so:
2. Does anyone know how to add to the config these defaultHeadings so they will work?

Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

Hi, finding out what I can do to help to move this forward - I'm planning on needing this functionality in another contrib module I'm working on. I'm not as well versed in these kinds of migrate plugins so I'm trying to follow your comment about deferring to getSourceUrls(). I see that function in there, but it's not being used anywhere. And not sure how to change the configuration piece either?

What if we made this a separate plugin that is a different name but extends the URL plugin and that way the original class isn't changed but the new one can add this functionality?

Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

Just FYI, I needed to update from 1.16 to 1.18 and the original patch we had didn't work for the new version. So, I removed the patch and decided to test this issue and it all seemed to work fine. Was this fixed from 1.16 to 1.18?

šŸ‡ŗšŸ‡øUnited States codechefmarc

Fixed as of version 5.0.0.

šŸ‡ŗšŸ‡øUnited States codechefmarc

This has now been fixed as part of 5.0.0.

šŸ‡ŗšŸ‡øUnited States codechefmarc

This has now been fixed as part of version 5.0.0.

šŸ‡ŗšŸ‡øUnited States codechefmarc

This has now been fixed in version 5.0.0. Thank you!

šŸ‡ŗšŸ‡øUnited States codechefmarc

@mandclu - thank you! That did it! I'm now running with the patch from MR 107 and that "Reduce output duplication" and that worked - no more errors.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Hi, I just tested MR107 on our setup, and I'm still getting the error:

Warning: Trying to access array offset on value of type null in Drupal\smart_date\Plugin\Field\FieldFormatter\SmartDateFormatterBase::rangeDateReduce() (line 661 of modules/contrib/smart_date/src/SmartDateTrait.php).

We aren't using the normal Drupal formatter here, we're using Layout Builder and getting the data in a custom block, not sure if that helps. For now, I'll stick to the patch from before and/or explore a preprocess to see if I can fix. Any pointers where I need to target the preprocess? Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

Would like to mention that we added c as a format that we are using and I hope this makes it into the next version. If this doesn't make sense, let me know if we should use the preprocess method instead. Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

I just updated to version 4.1-rc6 and still had this issue. I didn't know of a good way to add a MR against that version since this ticket is meant for 3.7. So, I'm attaching a patch that will fix this for 4.1-rc6.

šŸ‡ŗšŸ‡øUnited States codechefmarc

I was just trying out this patch and noticed that if there isn't a value passed to the urldecode function on line 220, it will give an error message about a required parameter. I suggest adding the following:
$value['uri'] = $uri ? urldecode($uri) : ''; or $value['uri'] = $uri ? urldecode($uri) : NULL; - I haven't dug too deep into if the $value['uri] needs to be an empty string or a NULL.

šŸ‡ŗšŸ‡øUnited States codechefmarc

I just did a test and I can do something like this and it works - not sure how valid it is though:

Migration yml:

source:
  plugin: url
  data_fetcher_plugin: http
  data_parser_plugin: json
  urls:
    callback: my_custom_callback
  callback_arg: 'my amazing argument'

Callback function:

function my_custom_callback(MigrationInterface $migration) {
  $my_arg = $migration->getPluginDefinition()['source']['callback_arg'];
}
šŸ‡ŗšŸ‡øUnited States codechefmarc

We are also using this patch to great success. I am curious if it's possible to add an argument that can be passed to the callback as well. Our use case is that we have several migrations that all use very similar code, but have different endpoints to get different resources. So, I envision something like this:

source:
  plugin: url
  data_fetcher_plugin: http
  data_parser_plugin: json
  urls:
    callback: my_custom_callback
    argument: something-passed-here

And then in the callback function:

function my_custom_callback(MigrationInterface $migration, $argument) {

}

This may be another issue/MR, but wanted to see if anyone else had this kind of use case. At the moment, the workaround is for us to use multiple callback functions, one for each migration.

šŸ‡ŗšŸ‡øUnited States codechefmarc

I've started some work in an MR - it's not ready yet as it still has some problems, but I based it off of #17 and started adding some things of my own. Mainly, once someone adds a block with auto_entitylabel, it was saving the correct auto label in the database, but not setting that as the block administrative title. So, I added some code to the auto_entitylabel_prepare_entityform function that still gets a decorated entity but splits that into its own variable. Then, we can use the passed in $entity to get a default value (I'm specifically testing for the case AutoEntityLabelManager::OPTIONAL:.

This works ... sort of. You have to save the block twice for it to actually take in the UI when not previewing the content and instead layout builder only shows the administrative titles. See new screenshots:




So, what I think needs to be done is to play with the auto_entitylabel_entity_prepare_view and/or auto_entitylabel_entity_presave functions to make sure it's saving the admin title so layout builder can read it correctly? I'm not as well versed with the entity API, so I'm running a little bit out of my comfort zone but I'll still press on to see what else I can do.

As a second issue here, if I choose to hide the label field and auto generate, I get the %AutoEntityLabel% as previously reported. That's, I think, being provided on line 185 (in my MR below) so that's something separate to troubleshoot.

See here for my current changes (Note again, this is not ready for merging yet):
https://git.drupalcode.org/issue/auto_entitylabel-3065687/-/compare/8.x-...

šŸ‡ŗšŸ‡øUnited States codechefmarc

Thanks, @samit.310@gmail.com - Good patch, however, since I'd like to make these configurable anyways, I added some new code to the dev branch for a config form, save the strings as config, and added code for the translations so they work with core config translations. If you want to take a look, I'd love a second pair of eyes. I'm adding a few things to the dev branch like other fixes from other issues on here as well.

šŸ‡ŗšŸ‡øUnited States codechefmarc

codechefmarc ā†’ made their first commit to this issueā€™s fork.

šŸ‡ŗšŸ‡øUnited States codechefmarc

I just tried your patch, Simon, and it worked for me but only in the /admin/content/block area. It didn't work for me for when you're adding blocks inline inside layout builder itself.

I tried #17 as well and it only sort of works for me - the default value for the admin label isn't getting set appropriately. I'm using the "Automatically generate the label if the label field is left empty". It will set the value correctly in the DB, but not in the admin label field, so (for example we have an Accordion block) it will put the block name in the field instead of what we put as the auto label, so it will put "Accordion". I think I found a fix for this, but, the preview text doesn't work yet, so I'm still working through that piece.

šŸ‡ŗšŸ‡øUnited States codechefmarc

These were fixed in the 4.0.0 release here - https://github.com/emulsify-ds/emulsify_twig/pull/55, but thank you for providing the patch, @sidharth_soman. Agreed about the readme, will keep it as is for now.

šŸ‡ŗšŸ‡øUnited States codechefmarc

This has been added in the 4.x version of the module. Closing this ticket out.

šŸ‡ŗšŸ‡øUnited States codechefmarc

I've added a fix based on some other issues also reported to the PR over on GitHub (https://github.com/emulsify-ds/emulsify_twig/pull/61) that should address these three Drupal.org issues:

https://www.drupal.org/project/emulsify_twig/issues/3160391 šŸ› Check if 'attributes' index is set in bem context Needs review
https://www.drupal.org/project/emulsify_twig/issues/3210140 šŸ› Error offsetExists() when i use 'twig_render_template' function Needs work
https://www.drupal.org/project/emulsify_twig/issues/3302662 šŸ› PHP error on passing array in attributes variable with add_attributes() Needs work

If someone can review it and let me know if that works, I'll merge that in. Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

I've added your fix from the other d.o. issue, @seanB, to the PR over on GitHub (https://github.com/emulsify-ds/emulsify_twig/pull/61) that should address these three Drupal.org issues:

https://www.drupal.org/project/emulsify_twig/issues/3160391 šŸ› Check if 'attributes' index is set in bem context Needs review
https://www.drupal.org/project/emulsify_twig/issues/3210140 šŸ› Error offsetExists() when i use 'twig_render_template' function Needs work
https://www.drupal.org/project/emulsify_twig/issues/3302662 šŸ› PHP error on passing array in attributes variable with add_attributes() Needs work

If someone can review it and let me know if that works, I'll merge that in. Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

I've added your fix (from the other d.o. issue), @seanB, to the PR over on GitHub (https://github.com/emulsify-ds/emulsify_twig/pull/61) that should address these three Drupal.org issues:

https://www.drupal.org/project/emulsify_twig/issues/3160391 šŸ› Check if 'attributes' index is set in bem context Needs review
https://www.drupal.org/project/emulsify_twig/issues/3210140 šŸ› Error offsetExists() when i use 'twig_render_template' function Needs work
https://www.drupal.org/project/emulsify_twig/issues/3302662 šŸ› PHP error on passing array in attributes variable with add_attributes() Needs work

If someone can review it and let me know if that works, I'll merge that in. Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

I've added your fix, @seanB, to the PR over on GitHub (https://github.com/emulsify-ds/emulsify_twig/pull/61) that should address these three Drupal.org issues:

https://www.drupal.org/project/emulsify_twig/issues/3160391 šŸ› Check if 'attributes' index is set in bem context Needs review
https://www.drupal.org/project/emulsify_twig/issues/3210140 šŸ› Error offsetExists() when i use 'twig_render_template' function Needs work
https://www.drupal.org/project/emulsify_twig/issues/3302662 šŸ› PHP error on passing array in attributes variable with add_attributes() Needs work

If someone can review it and let me know if that works, I'll merge that in. Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

I've tested your fix, seanB, and that appears to work for me. I'm going to do some additional testing before I put in a PR over on GitHub where we manage this module. Thanks for submitting the bug and the fix. I'll post again when the PR is ready for review.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Following up on this one. I'm trying to recreate your issue and I'm not running into that error. Can you show what code is in your twig template that is causing this? I've tried the following:

<div{{ add_attributes(['first', 'second']) }}>

Produces:

<div 0="first" 1="second">

And:

{% set text_field__attributes = {
    'my-test-attribute': text_field__width|default(['first', 'second']),
  } %}

  <div{{ add_attributes(text_field__attributes) }}>

Produces:

<div my-test-attribute="first second">

šŸ‡ŗšŸ‡øUnited States codechefmarc

Cleaning out some older issues here. I believe this was fixed in the 2.0.1 release and is also available in the 4.0.0 (current) release. See here:

https://github.com/emulsify-ds/emulsify_twig/pull/28

If this is still a problem, please open a new issue. Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

Cleaning up older issues here.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Cleaning out some older issues in the queue and I tested this one on Drupal 10 with the contrib version of Quick Edit (1.0.3) and it seems to be working just fine. Feel free to open a new issue if you are still having problems with this. Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

All Drupal 9/10 compatibility has been addressed in https://www.drupal.org/project/emulsify_twig/issues/3301353 šŸ“Œ Update emulsify_twig to not use pre-Twig 2.7 base class names Fixed .

šŸ‡ŗšŸ‡øUnited States codechefmarc

An update for my previous comment - I applied a patch based on this commit: https://git.drupalcode.org/issue/drupal-2999491/-/commit/1330eff91b12349... and that seemed to work great as it provides the required class LayoutReusableBlockDiscardChanges.php. I'm still testing it out and will report any bugs if I find any.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Dev version has been updated with the phpcs changes. Thanks for reporting! I'll be making some further changes to the dev version before merging in.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Updated patch is much better. This one still has a fallback image for reusable blocks that are based on inline blocks that don't have an image specified, but if the original inline block has an image specified, it will use that image in the reusable area as well rather than being generic.

šŸ‡ŗšŸ‡øUnited States codechefmarc

I was able to test this one out and it works great! In looking at the code, do you need a getter/setter for your new fields in LayoutBuilderBrowserBlockCategory.php? I'm not as well versed in custom entities so not sure if that is required there or not.

I have another patch which adds a generic fallback image to blocks that don't have an image or for reusable blocks at the bottom, if enabled. I tested both patches together and they work perfectly well to allow for category fallbacks and if they aren't set, the generic fallback works. If both of these get merged in, we should also change the language on the forms a bit to make sure that folks know which fallback image will be used depending on what is set.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Hi there, we're running 10.1.6 and I tried the patches from #103 and #82, and neither worked for me. I was getting the same error as some others above:
Class "Drupal\layout_builder\LayoutReusableBlockDiscardChanges" not found

I see that the later patches / MR are against D11. Will this work with D10 at all or do we have to wait until D11 for this one? Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

Came across this and GaƫlG's worked great when there was a link set. If one was not set, it would cause errors. So, I modified one part of it:

function MY_MODULE_save_menu_link_fields(array $form, FormStateInterface $form_state) {
  if ($link = _ys_core_get_link($form_state)) {
    // Only save the menu item extras if there is a menu link with a route.
    if ($link->getUrlObject()->getRouteName()) {
      $form_display = EntityFormDisplay::load('menu_link_content.' . $link->getMenuName() . '.default');
      assert($form_display instanceof EntityFormDisplay);
      $form_display->extractFormValues($link, $form['menu']['link']['extra'], $form_state);
      $link->save();
    }
  }
}

The change is the if ($link->getUrlObject()->getRouteName()) { which checks to see if there is an actual route in the link. If so, go ahead and save the menu item extras fields. If not, ignore it.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Just realized that if we make this ^6 for the Symfony version, that it won't work for Drupal 8 or 9. So, since this would be a breaking change, perhaps a new 2.x version of this module for Drupal 10 only?

šŸ‡ŗšŸ‡øUnited States codechefmarc

Hi HasinaNjaratin, I'm running into this as well. You mentioned that the fix is to get the right Drupal core version. I've got the following in my root composer.json file:

"drupal/core-composer-scaffold": "^10",
"drupal/core-project-message": "^10",
"drupal/core-recommended": "^10",

Which should get us 10.1, but I'm still getting errors like you were. What did you eventually put as your Drupal version to make this work? Thanks!

šŸ‡ŗšŸ‡øUnited States codechefmarc

I've tested both patch #4 and patch #8. Both worked to re-order the images and save in the correct order, however patch #8 worked better for me as it continued to show a preview of the image (pre-save) whereas #4 required a save before I could see a preview.

šŸ‡ŗšŸ‡øUnited States codechefmarc

Yes, I've gotten it to work like this:


    $form['my_tabs'] = [
      '#type' => 'horizontal_tabs',
    ];

    $form['tab_content'] = [
      '#type' => 'details',
      '#title' => $this->t('Tab Content'),
      '#open' => TRUE,
      '#group' => 'my_tabs',
    ];

    $form['another_tab'] = [
      '#type' => 'details',
      '#title' => $this->t('Another Tab'),
      '#group' => 'my_tabs',
    ];
    
    $form['tab_content']['my_field'] = [
      '#type' => 'textfield',
      '#title' => $this->t('An awesome textfield'),
    ];

šŸ‡ŗšŸ‡øUnited States codechefmarc

We use Config Split ( https://www.drupal.org/project/config_split ā†’ ) for this purpose and seems to work really well. That said, I'd love the module to know if it's running on Pantheon vs locally and adjust accordingly.

šŸ‡ŗšŸ‡øUnited States codechefmarc

An update for you and my solution. I don't think this is a bug - I think the module is working as intended, it was just a bit tricky to figure out how to update existing, overridden, layouts. We can close this if you wish.

So, here is my solution I did in an deploy hook and it worked beautifully:

$nids = \Drupal::entityQuery('node')->condition('type', 'page')->execute();

    foreach ($nids as $nid) {
      $node = Node::load($nid);
      $layout = $node->get('layout_builder__layout');

      /** @var \Drupal\layout_builder\Field\LayoutSectionItemList $layout */
      $sections = $layout->getSections();
      foreach ($sections as $section) {
        if ($section->getLayoutSettings()['label'] == 'Content Section') {
          $section->unsetThirdPartySetting('layout_builder_lock', 'lock');
          $section_storage = $this->getSectionStorageForEntity($node);
          $tempStore = \Drupal::service('layout_builder.tempstore_repository');
          $tempStore->set($section_storage);
          $node->save();
        }
      }
    }

We needed to remove all layout builders lock settings, so this worked. But if anyone else needs something similar, another method would allow you to programmatically set any third party settings. See here: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Config%21...

šŸ‡ŗšŸ‡øUnited States codechefmarc

This was what we were looking for too, however, we also needed the `c` format for "ISO 8601 date" so here is my updated patch that also includes that one.

Production build 0.71.5 2024