🇺🇸United States @maskedjellybean

Opened a MR but feel free to close since you've said this works as designed. I wanted to create a patch for myself and anyone else who would like the test webform route access settings/rules respected.

Attaching patch from the MR.

Does anyone else feel like this needs higher priority?

I've realized that the problem is not necessarily with \Drupal\date_recur\DateRange, but with the lack of form validation on the field widget which would prevent a user from encountering the exception.

Luckily I'm using date_recur_modular and found a patch that adds that validation to the Alpha widget. I rerolled the patch and opened an MR there.
https://www.drupal.org/project/date_recur_modular/issues/3071666#comment... 🐛 No form validation if start date is greater than end date Needs work

MR opened. I can see that the build looks like it will fail with an error that Drupal\date_recur_entity_test\Entity\DrEntityTest does not exist. I can say locally with a feature branch off of 3.2.x that this class does exist and Drupal\date_recur_entity_test\Entity\DrEntityTestBasic does not, which is why I "fixed" this in my MR. Not sure what's going on there.

Nevermind! Updating behat/mink solved all issues. That's because sometime between 1.10.0 and 1.12.0 \Behat\Mink\WebAssert::addressEquals and \Behat\Mink\WebAssert::addressNotEquals were properly type hinted. In 1.10.0 the arguments are not type hinted leading to this error when used in combination with Drupal core 10.4.5.

I found that updating behat/mink to 1.12.0 fixes a similar declaration error for \Drupal\Tests\WebAssert::fieldValueEquals but does not solve the issues for \Drupal\Tests\WebAssert::addressEquals and \Drupal\Tests\WebAssert::addressNotEquals.

I suspect these changes need to be rolled back. All documentation I've found for overriding methods in PHP suggest it's not possible to do what was done here.

This is causing PHPUnit tests to fail in Drupal core 10.4.5 and behat/mink 1.10.0. Tests are failing with this error:

"Fatal error: Declaration of Drupal\Tests\WebAssert::addressEquals(Drupal\Core\Url|string $page) must be compatible with Behat\Mink\WebAssert::addressEquals($page) in /app/docroot/core/tests/Drupal/Tests/WebAssert.php on line 775"

Even PhpStorm flags this as an issue. Not sure how this is not causing trouble for anyone else.

Fix inconsistent naming of webroot directory in first example.

Is the first example really limited to PHPUnit 9 + Drupal 10? I'm not so sure.

Support running test classes that extend BrowserTestBase.

maskedjellybean → created an issue.

I do think there is an action to take here. If the "Test webform" details section of webform access settings doesn't do anything and that is expected behavior, shouldn't it be removed?

Coming back to this with another finding in case it's helpful for someone. Another global permission that will unexpectedly cause the Test tab to be visible is "View webform submissions for any node".

@teknocat Any chance you could post your work around? Paragraphs UI is so clunky. I'd love to improve it even if it's through custom code/templates.

This resolves the warning for me but I'd say a more clear way to go about it would be:

if (isset($element_to_set_error['#type']) && $element_to_set_error['#type'] == 'hidden') {

Opened an MR with a fix for this. The issue is not really related to the autocomplete widget, but instead an autocomplete widget that accepts multiple values which is then automatically rendered inside a table (tabledrag). Specifically this fixes the issue in Claro. It may fix the issue in other themes as well, but since we can't control the markup of the table header (because that is up to the theme) I want to call out Claro specifically as the only theme I've tested this in.

maskedjellybean → made their first commit to this issue’s fork.

maskedjellybean → created an issue.

This issue now contains a couple related/duplicate fixes: https://www.drupal.org/project/field_group/issues/3160987 🐛 Form Required Class Missing from Claro RTBC

• Resolves missing asterisk for a Tab field group nested inside a Details field group (the Tab is technically a <details> element and there is nothing preventing a user from doing this).
• If the theme is Claro, fixes missing asterisk on <fieldset> by adding .form-required class to .fieldset__label inside the <legend>.

I don't believe the patch we have in this issue is really related to nested field groups. Required asterisks are missing from all <fieldset> in Claro without this patch or the one from the issue I linked, not just nested ones.

I fixed missing required asterisks in <details> and <fieldset> field groups in Claro over on the field_group issue. See my comment: https://www.drupal.org/project/field_group/issues/3160987#comment-16119890 🐛 Form Required Class Missing from Claro RTBC

Also pushed a fix for missing required asterisk on <fieldset> in Claro.

I came around to the conclusion that ensuring other types of field groups get the required asterisk is the responsibility of Field Group since Field Group adds the .form-required class in the first place. How should Claro know about a class that Field Group added? So I came up with a fix for missing required asterisks in Details and Details Sidebar field groups.

It wasn't possible to do using only CSS because of the markup like I showed in #45. Additionally I found that when I looked at details.html.twig provided by Claro, the template can optionally insert a span meant for adding the asterisk. It's beyond me to say why the template is not doing this, but my fix inserts it via JS if it wasn't added by the template. When the span exists it looks like this:

Unrelated to Claro I also fixed a Tab field group inside of a Details field group not getting the required asterisk. Tab field group is technically a <details> element. There's no form validation when editing form display to prevent someone from putting a Tab inside a Details, so we should make sure it gets marked as required if necessary.

Hey all, we have a fix for missing required asterisks in horizontal and vertical tabs in Claro in the works in field_group 4.x:

https://www.drupal.org/project/field_group/issues/3160987 🐛 Form Required Class Missing from Claro RTBC

It is really the same solution as you have in #99.

Copying and pasting my comment from that issue:

"I was also looking at https://www.drupal.org/project/drupal/issues/3171835 🐛 Field Groups marked as required are missing red asterisk RTBC which is marked as related. Looks to be trying to solve the same issue but in Claro/core. My thinking is that since Field Group provides horizontal tabs it should be responsible for making sure they work in core themes. Technically vertical tabs are part of core so I suppose we shouldn't really be fixing them, but since we can do it so easily I think we should.

However I'm seeing that other types of field groups don't have a required asterisk either, like Details and Fieldsets. These are such basic HTML elements that it seems to me that core should be responsible for making sure they have correct styling. Even if we wanted to fix them in field_group, there isn't enough markup to do it. You can see that summary.form-required already has an ::after pseudo element, so we can't add the asterisk without breaking styling:

"

That said, I'm now realizing that .form-required is only added to <detail> because of field_group. In that case maybe all of this is the responsibility of field_group. But then again, it's not possible to style it without modifying a template provided by Claro...

Oh, good call on html[data-once~="claroDetails"]. Looks like that will work!

Thanks for the go ahead. I'll modify the same branch today.

I don't use Gin so I'm not cut out to fix the issue you linked but maybe it does require a similar solution.

I was also looking at https://www.drupal.org/project/drupal/issues/3171835 🐛 Field Groups marked as required are missing red asterisk RTBC which is marked as related. Looks to be trying to solve the same issue but in Claro/core. My thinking is that since Field Group provides horizontal tabs it should be responsible for making sure they work in core themes. Technically vertical tabs are part of core so I suppose we shouldn't really be fixing them, but since we can do it so easily I think we should.

However I'm seeing that other types of field groups don't have a required asterisk either, like Details and Fieldsets. These are such basic HTML elements that it seems to me that core should be responsible for making sure they have correct styling. Even if we wanted to fix them in field_group, there isn't enough markup to do it. You can see that summary.form-required already has an ::after pseudo element, so we can't add the asterisk without breaking styling:

I'll make a comment on the core issue after I modify the MR.

@dimitriskr I'm looking at this today too! Thanks for the MR! It resolves the issue for me but I have a couple ideas for improvements.

I know the issue title says horizontal tabs, but vertical tabs in Claro are also missing asterisks. If we could solve for both that would be ideal.

I may create another branch in the fork that creates a formatters/tabs.css which contains the fix for both horizontal and vertical tabs in Claro:

/* Fix missing required asterisks in Claro */
.horizontal-tabs .form-required::after,
.vertical-tabs__menu .form-required::after {
  display: inline-block;
  margin-inline: 0.15em;
  content: "*";
  color: var(--color-maximumred, #dc2323);
  font-size: 0.875rem;
}

This is slightly different from your CSS. I don't think the selectors need to be quite so specific because they will be harder for someone to override if necessary and more prone to breaking if field_group changes the markup of tabs slightly. Also we can set #dc2323 as the fallback color to be used in the case where the --color-maximumred variable is unset (which I assume would be all themes except Claro).

Creating a formatters/tabs.css makes sense to me because formatters/tabs.js is what adds the .form-required class to the tabs. So tabs.css could be thought of as the associated styling.

I do wonder whether we risk messing up the styling for asterisks in other themes. Should we be taking this a step further and only loading our Claro fix if the base theme is Claro? This is such a weird situation.

I think this is a duplicate of https://www.drupal.org/project/field_group/issues/2969051#comment-15985788 🐛 HTML5 Validation Prevents Submission in Tabs Postponed: needs info and is resolved in 4.x.

@anybody I noticed that you set this issue to version 4.x-dev but the MR merges into 8.x-3.x. I'm looking to solve this issue in 4.x. Does the MR need to be rewritten for 4.x-dev?

I found the same as @scott_euser in #177: Upgrading to field_group 4.x resolves the issue as seen in the gif.

Tabs are still not marked as required, but that's a separate issue I'm sure.

maskedjellybean → created an issue.

maskedjellybean → created an issue.

Oh wow. Today I learned we are still on CKEditor4. Thank you!

Please feel free to close this issue.

maskedjellybean → created an issue.

maskedjellybean → made their first commit to this issue’s fork.

I found you also get this error if you have the wrong base_url in behat.yml. Hope this helps the next person.

Ok, updated the MR.

We can support index I think. Might as well add support for all the currently unsupported URL patterns in this issue. Also we were regexing for v (VIDEO_ID) and doing nothing with it, so I tried to address that.

From what I've seen, URLs in those 3 patterns now work, however I see no change in the first video played no matter what. My assumption is that in \Drupal\video_embed_field\Plugin\video_embed_field\Provider\YouTubePlaylist::renderEmbedCode we should set $embed_code['#query']['v'] and $embed_code['#query']['index']. At the very least doing this does not cause any issues loading the playlist.

Created a merge request with correct regex. Both "/watch?" and "/playlist?" should work.

I'm not sure that we actually need this part of the regex: (?=.*v=(?<video_id>[0-9A-Za-z_-]*)), so I made it optional. It seems like this would only be relevant for a URL to a single video.

maskedjellybean → created an issue.

Thinking about it, I'm unsure why we would check for $element['table'][$item]['enabled']['#value'], because doesn't the fact that the item is in the table at all indicate that it should be saved as a field value?

I understand now based on the screenshots on the module description the use case for combining checkboxes with tabledrag. I updated the MR to check for whether the field is an entity reference and whether it uses a view as the reference method. My thinking is that if the reference method is a view, there can be no checkboxes, therefore we don't care whether $element['table'][$item]['enabled']['#value'] is empty.

maskedjellybean → created an issue.

I tested full duplex using 4.0.0-alpha7 and found it does not work in either direction.

On initial page load of the edit form the map does not match the Address field. It zooms in on the South Atlantic Ocean.

Modifying the Address field values does not change the map pin.

Clicking on the map does not change the Address field values.

The console gives this JS error:

GeolocationMapBase.js:597

InvalidValueError: <gmp-internal-am>: Cannot set property "content" to 1: not an instance of Node; and 1 InvalidValueError: <gmp-internal-am>: Cannot set property "content" to 1: not an instance of Node; and 1
    at Object.Cj (main.js:147:373)
    at QP.ph (main.js:373:248)
    at set content (marker.js:121:177)
    at new GoogleMapMarker (GoogleMapMarker.js:19:33)
    at GoogleMaps.createMarker (GoogleMaps.js:123:12)
    at GeolocationDataLayer.js:118:31
    at NodeList.forEach (<anonymous>)
    at DefaultLayer.loadMarkers (GeolocationDataLayer.js:117:49)
    at DefaultLayer.loadMarkers (DefaultLayer.js:6:18)
    at GeolocationMapBase.js:591:22 Loading '/modules/contrib/geolocation/js/DataLayerProvider/DefaultLayer.js' failed

Ah perhaps I'm in the wrong place then. I'll open a different issue. The problem I'm trying to solve is:
1. Views does not support filtering on a datetime database field.
2. Views does not support displaying a datetime database field.

I realize the status of this is postponed, but the fix for issue as I understand it seemed doable so I opened a MR.

This is what I think the goal of this issue is (correct me if I'm wrong):

If you have a base table (in Drupal speak, apparently this means a table unrelated to any entity) that has a datetime field/column, it should be filterable in views the same way that a timestamp integer field would be. Additionally the datetime field should be displayed in the view the same way that a timestamp field can be displayed.

If that's correct, then the MR does the trick for me.

For example, given this:

/**
 * Implements hook_schema().
 */
function my_module_schema() {
  $schema['my_custom_table'] = [
    'description' => 'A custom table.',
    'fields' => [
      'date' => [
        'type' => 'varchar',
        'mysql_type' => 'datetime',
        'not null' => TRUE,
      ],
    ],
  ];

  return $schema;
}

/**
 * Implements hook_views_data().
 */
function my_module_views_data() {
  // Make my_custom_table table
  // queryable by views.
  $data['my_custom_table'] = [
    'table' => [
      'group' => t('A group'),
      'provider' => t('my_module'),
      'base' => [
        'field' => 'date',
        'title' => 'An example table',
        'help' => 'This is an example table description.'
      ],
    ],
    'date' => [
      'title' => t('A datetime field/column'),
      'help' => t('This is a datetime field/column.'),
      'field' => [
        // ID of field handler plugin to use.
        'id' => 'datetime',
      ],
      'sort' => [
        // ID of sort handler plugin to use.
        'id' => 'datetime',
      ],
      'filter' => [
        // ID of filter handler plugin to use.
        'id' => 'datetime',
      ],
      'argument' => [
        // ID of argument handler plugin to use.
        'id' => 'datetime',
      ],
    ],
  ];
}

I can make a view of "An example table", and add a "A datetime field/column" field and filter.

maskedjellybean → made their first commit to this issue’s fork.

maskedjellybean → created an issue.

Wow, thank you @annmarysruthy! I didn't expect to have a fix so quickly. A patch from the MR fixed the issue! The test seems to make sense. Thanks again.

maskedjellybean → created an issue.

I can replicate this issue with Drupal core 10.3.9 and same_page_preview 2.1.4.

It would be very nice if this worked, because changes to fields are not always detected. Triggering the preview manually would be a kind of work around.

Remove "9" from title. I don't think there's a reason to include version numbers in docs except for "Drupal" and "Drupal 7". It's needlessly confusing for new users. If drupal.org wants to have different docs per version post Drupal 8, then they should implement a real docs system.

@dpi I think you might be wrong about that. I'm using Alpha and it looks like I could disable every ends mode option except infinite ("Never"). I tried unsetting Never in a form alter, but that ends up causing other issues because if I choose "On date" and save, then the next time I edit the node "After number of occurrences" is selected.

maskedjellybean → created an issue.

maskedjellybean → created an issue.

maskedjellybean → created an issue.

#25 worked for me as well. Thanks!

Thanks for moving this to the correct project.

Searching for "Subscriber" is not what I'm looking for. That will give you existing event subscribers, not events that can be subscribed to. You could find events that can be subscribed to that way, but it would be very time consuming. Plus it's theoretically possible for core to provide an event that is not subscribed to by core, meaning you wouldn't find it this way.

I have seen https://api.drupal.org/api/drupal/core%21core.api.php/group/events/11.x , but it's not accurate. For example \Drupal\file\Validation\FileValidationEvent is not listed. Is this list automatically generated or manually maintained?

maskedjellybean → created an issue. See original summary → .

Fixing missing "to". Remove "web" from example command.

Move Lando documentation to a separate page because it was getting too long.

Fix title missing "s".

Change headings.

Update Lando custom tooling command to support both D10 and D11 despite the syntax changes introduced to phpunit.xml.dist in D11.

Update custom Lando tooling phpunit command to work in Drupal 11.

New patch reflecting latest changes in MR:

• Fix broken iCalTimezoneGeneratorTest. All tests now pass, at least locally.
• Fix use of array_shift() seemingly accidentally causing empty array by removing first element, which then caused no transitions to be generated. Update test to reflect new generated calendar string.
• Add test for generated calendar for a recurring event (Compares generated ICS file string to an expected string containing RRULE property which has been validated by https://icalendar.org/validator.html).
• Remove the addition of datetime and daterange to supported field types array because it is completely untested and seems outside the issue scope.

The use case is if you want to index more than 10000 characters in one record. :-)

Algolia offers the ability to split records in order to get around their character count limitation, so it would be great if search_api_algolia leveraged this ability.

Potentially site builders/developers may not realize their records are being truncated. When it is truncated search does not search the entire record because only part of it is indexed. This means worse search results without any indication why.

Wow, #20 was the cause of my error as well. No idea how you figured that out based on the error message but I appreciate it!

After applying the patch from the MR and adding the patches-ignore from #6, try deleting /vendor and /docroot/modules/contrib/lightning_core and then running composer install.

maskedjellybean → created an issue.

Here's a version of the patch for Group 1.0.x (tested on 1.6.0) that combines the patch from #12 with the patch from #3 of this issue: https://www.drupal.org/node/3364703 →

The two are unrelated, but since they touch the same code, you can't apply both at once. This is likely something that only a few people would run into and probably frowned upon by maintainers, but uploading anyways. All this does differently from #12 is reload the entity in order to check that it still exists before saving. Strangely the code comments around line 216 of GroupContent.php imply that this is already happening, but it's clearly not.

Wow thank you for this! In my case this error was being thrown during a Behat afterScenario while attempting to delete nodes. Here is a version of the patch for Group 1.0.x (tested against 1.6.0) for those of us still stuck on a super outdated version.

That link is very informative, and thank you for writing it, but what I was trying to say is that we should clean up the documentation on drupal.org. The DDEV stuff is currently sprinkled all over the page instead of under a specific header.

But if there is a place to link to @selwynpolits site we should do it. Maybe a More Resources header?

Hey @JoseFran, you can generate a patch at: https://git.drupalcode.org/project/hook_event_dispatcher/-/merge_request... (just add .patch to the merge request URL).

@murz, can you explain how to use this feature once the patch is applied? I'd like to implement hook_ENTITY_TYPE_view.

I added a related issue that was seemingly incorrectly closed. Just to reiterate what was pointed out in that issue, there is almost certainly a performance penalty for using hook_entity_view when really you only need hook_ENTITY_TYPE_view. The first is called when viewing any entity and the second is only called when viewing a specific entity type. If events are going to be as performant as hooks then this needs to be figured out.

I have this problem but I'm not using the private file system (only public), so the patch doesn't help me. I tried the event subscriber in #3, and while it did work, it seems a little overbearing to run that if statement on every page request across the site on the off chance that the request contains "csv". Adding to .htaccess as suggested in #2 didn't work for me.

In the end I did what was suggested by the reporter: Set the file name in the views path settings.

This is a bug for sure, but at least it's easily worked around.

jwilson3 → credited maskedjellybean → .

I was experiencing this with Composer version constraint ^2.0@RC. Upgrading to ^3.0@RC appears to have fixed it. No patch was needed.

Wonder if someone who uses DDEV would move all the mentions of DDEV underneath the DDEV header. I think it would help folks navigate this easier, even if it means repeating some info.

Thank you for the patch. It fixed my issue importing into Outlook for Mac.

If you want to replicate the problem, use the module without this patch and try to import into Outlook for Mac. You'll see an error. Apply this patch, make a change to the node, save and repeat. No error.

If you don't have Outlook for Mac, you run a file generated by this module through the ICS validator:

https://icalendar.org/validator.html

You'll see errors like "Invalid TZOFFSETFROM".

Apply the patch, make a change to the node, save, download the file again and run it through the validator. You won't see that error anymore.

Realized my change to \Drupal\ics_field\ICalTimezoneGenerator::getMinMaxTimestamps was causing the file to be generated without BEGIN:DAYLIGHT, TZOFFSETFROM etc. Fixed with this patch.

This made me realize why my changes appeared to fix Outlook support. It's because prior to this patch I had accidentally removed the TZOFFSETFROM property. This module formats TZOFFSETFROM incorrectly, which Outlook (at least Outlook for Mac) then refuses to import. By accidentally removing that property entirely I had "fixed" Outlook. Now I realize there's a separate issue for this: https://www.drupal.org/project/ics_field/issues/3004699 →

The patch from the MR works for me in 10.1.8 to resolve this error thrown by a view. The view then functions as it should. Not sure what to make of that.

I think @MFH is right about this being potentially confusing. Shouldn't the title be "Create a custom plugin block"?

Made the requested changed to the MR. Unfortunately I can't do anything more to make the tests pass because the error appears to have nothing to do with the test being added here.

Removes unnecessary forward slash.

Capitalizes PHPUnit. Remove unnecessary language. Moves link to Lando under top Lando heading.

Adds example of Lando custom tooling command to run PHPUnit. Moves all mentions of Lando under the "PHPUnit in Lando" heading. I haven't done the same of DDEV because I don't use it. I'm not convinced that Lando and DDEV should be mentioned before the "Setting up to run PHPUnit tests manually" heading or that that heading is the best descriptor.

Thank you for the patch in #99. It still works in Drupal core 10.2.3.

I opened a MR (in case that helps move this along) which contains the changes from #99, plus hopefully fixes the tests, although I can already see they've failed...

maskedjellybean → made their first commit to this issue’s fork.

Opened a MR containing the changes from #4. I didn't include the later patches because they contain changes from https://www.drupal.org/project/ics_field/issues/2852239 ✨ Include end date / time in ICS file. Needs work .