🇺🇸United States @chrisolof

I found that what's outlined in comment 11 works to achieve something similar to what I think the OP was trying to do. In my case I needed to render the product full/default view with most product variation fields showing, and in product lists I needed a "teaser" type of output where only some fields were shown, sometimes with different output settings.

To achieve this I:

1. Set up same-named "Teaser" view modes (/admin/structure/display-modes/view) for both the Product and Product variation (enabled for the sub-types that needed it).
2. Configured the display settings for each, reducing and changing how certain fields were rendered in the teaser view modes via each Product/Product variation field's display settings.
3. Created both commerce-product.html.twig and commerce-product--teaser.html.twig template files with statements like {{- product.variation_field_foo -}} and {{- product.variation_field_bar -}} to render the fields where needed. It sounds like layout builder could be employed here as an alternative to this template-based approach (see #2716417-22: Allow rendering of variation fields on the product entity. → ).
4. Selected the "Teaser" view mode for products in views-driven product lists.

Seems to work great.

I agree with comment 14 that it would certainly be nice if we did not have to rely on this same-name magic and could instead select the desired product variation view mode from the product view mode's display settings (defaulting to current behavior).

This bug is a bad one for performance. On our site it was behind a very problematic "route" cache context bubbling onto many of our site's render arrays, causing lots of unnecessary re-rendering per route and a pile-up of identical render cache entries (again, per route). This has the effect of largely disabling the benefits of render caching, while at the same time ballooning render cache write operations and render cache storage needs.

I can confirm that rules-3161036-6-v2.patch from #6 solves the issue, and seems to do so in a way that preserves cacheability metadata from URL generation where needed without letting it bubble onto unrelated render arrays. It doesn't seem like it would be wise to just totally toss this metadata out, as the v1 patch does.

Merge request 12896 opened against 11.x. It contains the changes from the patch in comment 42 with a slightly improved post-update hook (param and return types declared + lower memory consumption by not loading all of the site's views in one go).

Needs review.

In our site this issue was contributing to render cache pile-ups for teaser/search result-type entity displays that lean on views for certain content (content that does not actually vary by URL).

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

@mlncn It looks like BEF builds its links from the current path (eg. /view/category/foo-258), not the view's path (eg. /view), and thus includes any active pretty path parts in all of the URLs it generates - even links that are supposed to remove selected options.

In other words it doesn't appear that BEF links (including the links widget) are fully-compatible with what we're doing here, yet. If BEF used the view's path (/view) as the starting point for its links, instead of the current path (/view/category/foo-258), I believe its links would become fully compatible.

You might consider proposing a patch to BEF that changes Drupal\better_exposed_filters\Plugin\BetterExposedFiltersWidgetBase::getExposedFormActionUrl() to pull the starting URL from $this->view->getUrl() instead of the current request. In a quick test in my dev environment I just confirmed that little change in BEF does indeed resolve the BEF links widget issue you describe.

I should also mention that moving to the checkboxes/radios BEF widget is another way to overcome the deselection issue, but that may not be right for your site.

chrisolof → created an issue.

I just ran into one more spot where the row's entity can live and have added a check for it in MR 21: $row['entity']['#' . $row['entity']['#theme']].

No luck with the patch. I swapped out Html::getId() for Html::getUniqueId(), which gave me a guaranteed page-unique wrapper ID. I then attempted to persist this unique wrapper ID in the form state for later use in subsequent AJAX requests. Unfortunately it seems that the initial form state is lost upon the first AJAX request, which meant the original wrapper ID was lost too (and the AJAX connection to that specific instance of the form with it).

It's possible someone more familiar with webform's AJAX system may be able to sort that out and somehow hang onto the wrapper ID. Maybe it's a bug that the initial form state is lost like that...

I just ran into this issue with webform 6.2.x. Two instances of the same AJAX webform on the page - both tied to the page's node as source entity. Both end up on the page with the same HTML ID in their form wrappers.

If I interact with the second webform instance (change to the next form page, for instance) nothing happens, but the first webform instance is driven instead. Basically we've got cross-talk on the AJAX channel between these two forms. Odd that they both come out with the same HTML ID. I hope to post a patch soon.

+1 for this change. This auto-field-adding (bug?) just bit me in a deployment.

The problematic deployment upgrades a content type's teaser display from the basic/regular type to a LB layout. When I deploy that change without this patch I get the new LB layout in place, as configured, but with all of the old fields from the old basic/regular display unexpectedly appended into the first section (result not matching config). When I apply this patch and repeat the same deployment test I get the new LB layout, as configured, with no surprise fields appended (result matching config).

MR 125 + MR 127 is working well in my tests.

It allows me to easily exclude certain content entities from the index based upon a field value. Throwing a SkipElementException is a standard and familiar way of achieving something like this. It also makes for really readable and easy-to-understand code in my hook, which I feel is a benefit over the set-entity-to-NULL approach discussed above.

Thanks for adding this!

@rob.sipek, it sounds like for your use-case you'd need to stick with the default, "Visually hide" filter removal strategy so that current filter values are maintained regardless of which block's form is submitted. In your use-case that hidden markup is necessary as it makes that behavior possible.

In some of my use-cases I do not need or want the hidden filters' current selections maintained, so the "Remove" strategy is a better fit (for example I have a search box that always initiates a fresh search - current deeper filtering choices are totally unnecessary and not desired in that block).

Maybe a little help text under the 'Filter removal strategy' field and options would make it easier for users to determine which strategy is right for their needs. Example:

Filter removal strategy
- Visually hide
   Allows for current selections/values in hidden filters to be maintained through submit.
- Remove
   Less markup, but only values from the visible filters in the form are submitted.

Keep the default, "Visually hide", if you're unsure or the "Remove" strategy is not working for you.

In testing I found that the pretty path redirect response to the views exposed form submission was coming back with "no-cache" in the cache control header. Adding cacheability metadata to the pretty path redirect response seems to have solved the issue. This should improve performance on previously-requested facet filter combinations.

Attached is an immutable patch against 2.0.x representing the current state of MR 20.

Branch 3516724-views-boolean-filter-null-bug under this issue's drupal-3516724 fork now includes a fix for this bug.

Still needs test coverage, so MR not yet opened.

The fix was to anticipate and handle the possibility of NULL/empty field values when setting up the WHERE clause in the query. Relying on a condition like [field-value] <> 1 is not enough, because that check will evaluate to FALSE if [field-value] is NULL in database backends like MySQL (see ref). Expanding the condition to ([field-value] <> 1 OR [field-value] IS NULL) resolves the issue by handling this gotcha with NULL in comparisons.

The attached patch represents the 3516724-views-boolean-filter-null-bug branch's current diff against 11.x.

Proposed resolution in the issue description updated to align with the fix that is proposed here.

chrisolof → created an issue.

Thanks edsoncarlos! Looks like we missed that over in 🐛 Deprecated FormElement Active . Issue filed: 🐛 Bump up minimum Drupal version to 10.3 Active

chrisolof → created an issue.

chrisolof → created an issue.

Wow. That is a problem. Thank you for catching this. I am re-categorizing this issue as a bug report.

I lean toward adding this new check in Drupal\phone_number\PhoneNumberUtil::testPhoneNumber() because that is a centralized checker used by both Drupal\phone_number\Plugin\Validation\Constraint\PhoneNumberValidator::validate() (line 69, $phone_number = $item->getPhoneNumber(TRUE);) and Drupal\phone_number\Element\PhoneNumber::phoneNumberValidate() (line 244).

That would ensure this new check is called in (I think) all situations (render element usage, regular field usage, programmatic entity field usage, etc.) and probably avoid any duplicated error messages.

And to ensure this bug stays fixed, I think we should add a test confirming the fix. It could be as simple as submitting India + 999 into a phone number field, confirming that the submission is not accepted, and confirming the expected form validation error is displayed.

Thanks everyone! MR 21 has been merged into 2.0.x and tests are now passing in a Drupal 11 environment.

chrisolof → created an issue.

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

Thanks arwillame. I think this plugin would be more at home, and likely better maintained, in the GraphQL Compose module's GraphQLComposeFieldType plugins folder. I see a bunch of other plugins just like this one, covering all sorts of contrib field types, in there already:

https://git.drupalcode.org/project/graphql_compose/-/tree/2.4.x/src/Plug...

If the GraphQL Compose maintainers are not open to this new plugin going there then I would encourage you to post this up as its own module where you can easily keep it up-to-date and running right.

If you do manage to get this into GraphQL Compose or your own contrib module let me know and I'll link us up on this module's project page so people can easily discover the connection.

Thanks bogdan.racz. These changes have been merged into our 2.0.x branch.

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

Alternate/similar idea: Integrate with Country → and Flags → and lean on those for our country select field.

Our phone input is now a tel instead of a textfield (change merged into 2.0.x). Thanks everyone!

Looking at our existing validation it certainly seems like adding a call out to \libphonenumber\PhoneNumberUtil::isValidNumber() would duplicate existing, earlier checks that validate the entered number against the chosen country. I must be missing something. Could you provide more detail on how to reproduce the issue that this addition resolves?

I totally support this idea but I agree with tim-diels that we'd want to build against a stable release of the flags module, which does not exist yet.

Since we now have two options for country selection (Two-letter ISO country code and Flag) there is some potential for making the Flags module a suggested dependency that opens up the "Flag" country selection option if present.

Thank you! This change has been merged into the 2.0.x branch.

Attached is a patch against the current 4.x branch that brings in support for compound fields of type managed_file and phone_number.

I look at this patch as a temporary fix, but not what we'd want to merge into this module. I still think the constraint route outlined above in #7 is the way to go as it would bring in support for nearly all compound fields and potentially reduce this module's codebase at the same time.

MR 12 fixes the TypeError I was running into when copying layout builder layouts with inline content blocks. Additionally, the changes in MR 12 should allow for layout cloning on sites with or without the Block Content module (and with or without inline Block Content blocks in their source layouts).

This is ready for review.

chrisolof → created an issue.

chrisolof → created an issue.

chrisolof → created an issue.

Confirming MR 262 is working great in my tests.

+1 on the number field idea. It appears these hard-coded options are just carried over from the 2.x config form. Probably it makes for a nice clear "No limit" option chosen by default instead of an empty box. Maybe we'd need to modify this text:

*NOTE: setting to "No limit" sets the hard_limit value to "0", which depending on the search backend could lead to limited results. Try setting to the maximum value if you are experiencing this issue

To something like:

Leave empty for no limit.
Note: Some search backends will still apply a default limit when no hard limit is set. If this affects you, set this to an appropriately-high value or a value that indicates "no-limit" to your search backend.

It appears this new setting still needs to be added into the module's config schema (config/schema/facets.views.schema.yml).

Does this need test coverage?

chrisolof → created an issue.

Basic test coverage added in MR 15. Verifies the whole BEF filter wrapper is hidden when the exposed filter is configured not to show in a configurable views filter block. This is now ready for review.

Re-titled the issue as it seems we're all pretty specifically after BEF collapsible details element support in here.

MR 15 adds this support, taking a slightly different approach at the code level, hopefully making it easier to add support for additional filter wrappers in the future (should more requests like this come in) without repeating code.

Test coverage not added yet, so changing status back to needs-work.

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

chrisolof → changed the visibility of the branch 3509467-add-option-to to hidden.

chrisolof → created an issue.

Update: I have just achieved pretty paths atop facets exposed filters over in issue ✨ Support facets 3.x facets exposed filters Active under the Facets Pretty Paths module.

Test coverage and small improvements added (some identified while adding the test coverage). This is now ready for review.

Attached is an immutable patch against 2.0.x representing the current state of the MR 20.

chrisolof → created an issue.

chrisolof → created an issue.

Support added in the 3505618-support-facets-exposed-filters branch. Tested against a variety of facet filters with a variety of employed coders, single and multi-value selections. Seems to be working well at this point.

Pretty paths coder config is at the filter-level (it shows in the exposed filter options form). It is possible to combine pretty and regular query string facet filters (eg. "/view/category/foo-258?tag=33).

Filter order in the URL is currently just following the filter order already established in the view.

Still needs test coverage and likely some additional refinement - both hopefully coming in this week.

chrisolof → created an issue.

chrisolof → created an issue.

chrisolof → created an issue.

chrisolof → created an issue.

chrisolof → created an issue.

I also ran into this issue and the provided patch appears to fix it. Thanks khiminrm!

Thanks Liam. Test coverage added, covering the clean-up of webform_scheduled_email records with missing a plugin or a missing submission, both in schedule and send state (to ensure cleanup is working in cronSchedule() and cronSend()).

While I was in there I found some disabled test code that seemed disabled due to this orphaned record issue, so I moved, updated, and enabled that check also.

MR 595 removes orphaned webform_scheduled_email records when encountered in cron methods instead of trying to proceed with them and causing PluginNotFoundExceptions. This is ready for review.

Interestingly it seems there is some skipping and cleanup logic already in this submodule.

The solution cronSchedule seems to take currently (when the referenced webform submission is missing) is to delete the orphaned record from webform_scheduled_email and move on. Cleaning up the orphaned (and unusable) record upon discovery seems better than logging and requiring a site admin to manually clean up the table someday. As such, I'm updating the proposed resolution.

chrisolof → created an issue.

chrisolof → created an issue.

chrisolof → created an issue.

chrisolof → created an issue.

The attached patch represents the state of MR 9 at the time of this comment.

MR 9 opened. Builds on MR 3.

- Uses the module's existing 1.x plugin definition API
- Condenses the code a bit
- Expands support to entity reference revisions fields

Needs review and testing. Using this along with MR 8 (from ✨ Add ability to clone a layout builder layout field Active ) allows me to deeply clone layout builder layouts, including sub-entities referenced from layout builder content blocks inside of layout components. Pretty neat.

Like in MR 8, it's possible we could use this module's clone process to clone these referenced entities when configured, falling back to the Drupal core-provided clone. That was not necessary for my project so I haven't gone that far here, but it certainly seems like a nice-to-have for projects that might need to configure how these referenced entities are cloned.

I think the change in src/Plugin/FieldProcessorPluginBase.php is an unnecessary, breaking change that should be proposed in a separate issue. A change like that would normally go out in the next major version of a module so as not to break existing plugin implementations written against the 1.x plugin API.

Merge request created. Working so far across a variety of complex layouts in my initial tests.

- New "Copy layout" field processor plugin, only offered for layout builder layout type fields.
- Clones each section inside of the layout
- Clones each component inside of each section, giving each component its own UUID. UUIDs are not intended to be shared.
- If component is a layout builder-provided content block, the content block is also cloned (at the revision specified by the component). These blocks are not intended to be shared across layouts. Cloning these blocks prevents cross-talk issues between original and cloned layouts. Block clone is done content_entity_clone-style if clone settings are enabled for the block type, falling back to a core entity clone.
- Copy values plugin adjusted to not show up for layout builder layout type fields, as it is inappropriate for that field type.

Needs review and additional testing.

Attached patch represents the current state of MR8 at the time of this comment.

chrisolof → created an issue.

chrisolof → created an issue.

Thanks DrupalHunk! The changes in 057961e40decefada8c9b92066e5a5ab66087511 (from MR5) appear to resolve this issue.

MR5 also currently introduces a .gitlab‎-ci.yml‎ file that seems to be a start at running automated testing / linting / building (of which we have none yet).

I feel this isn't the right issue to bring in a .gitlab‎-ci.yml file, unless it also brings in a test confirming the fix here. I've opened ✨ Automate manual tests Active , which I think would be a good spot to bring in a .gitlab‎-ci.yml file.

chrisolof → created an issue.

chrisolof → created an issue.

The attached patch against 2.0.x:

- adds a sort into the mix so that entry order of inclusions and exclusions is no longer important
- adds documentation of how to exclude under the textareas where exclusions can be entered
- updates some code comments to make it clear that we're working with exclusions as well as wildcards

Tests are still needed here.

Hey guys - I believe we've got this in the latest beta by way of the fix to 🐛 Not properly working with Google maps provider Fixed .

See: https://git.drupalcode.org/project/address_autocomplete/-/blob/1.0.0-bet...

Due to the decoupled nature of this functionality this provider bypasses the stock Drupal autocomplete widget in favor of the one provided by the Google Maps API. This is both for ease of use purposes, and also because attempting to implement this API using the Drupal autocomplete widget results in large numbers of expensive (as in money) queries in order to function.

The current implementation utilizes autocomplete session tokens to avoid this, and I believe is then able to lean on the regular Drupal autocomplete functionality.

Unless I'm missing something here, I believe we can close this issue as a duplicate.

From what I can tell this bug is now fixed in the latest version of this module. Code similar to what's happening in #13 is now in place, which apparently helped fix ✨ Hide output if empty option Fixed . I don't have as many usages of EVAs in LB layouts anymore, but those I do have seem to be working with the latest version of this module (unpatched).

Can others still reproduce this issue with the latest version of this module?

What specifically does the error message say? Also - be sure you've allowed (toll-free) 1-800 numbers in your field instance settings and be certain the 1-800 number you're testing with is actually a Canadian 1-800 number.

The actual error message usually indicates pretty specifically if the issue is with the number type not being allowed in the field or a number that is not actually from the chosen country, which would be helpful in terms of understanding what specific validation issue you've run into here.

The attached patch resolves the issue by changing the scheduled email datetime format to ISO 8601, which includes timezone information at the end. WebformScheduledEmailTest is also updated to assert the new format is coming out as expected in messages.

chrisolof → created an issue.

TR → credited chrisolof → .

Notes from testing this MR against real traffic for about two weeks now:

This is currently performing very well / as designed against real traffic. The addition of the ASN lookup on those requests where it is actually necessary (requester not openly identifying as bot and not already blocked at the visitor-level) is so fast, even without the optional C extension, that it is imperceptible to our end-users. On the other hand, rate-limiting all crawlers, including those that horizontally spread out across multiple IPs and/or user agent strings, is a perceptible performance boost for our end-users.

Here's an initial patch that makes rescheduling after webform / source entity updates optional. Needs review.

chrisolof → created an issue.

I like this idea. I don't think number is the right type (we'd lose the excellent input format flexibility we have today), but I see no downsides to switching to tel. It sounds like it would have client-side UX benefits and supports all phone number formats (assuming we don't specify a pattern).

I think this also opens up some neat future possibilities. The patch looks fine but needs testing.

Also, related, over in 🐛 Undefined array key "extension" in /src/Element/PhoneNumber.php on line 237 Needs review I'm proposing we switch the extension sub-field type to number.

The attached patch resolves the issue in the affected environments.

I've made three changes in here:

1. Switch the extension sub-field type from text to numeric.
2. Remove the custom numeric extension validation code in Drupal\phone_number\Element\PhoneNumber::phoneNumberValidate() that is no longer needed.
3. Ensure the array key "extension" is actually set before trying to access its value in Drupal\phone_number\Element\PhoneNumber::phoneNumberValidate().

I suspect the third change is most responsible for eliminating the error here, and the patch could certainly be reduced to just that change. However while examining the validation of our extension sub-field it felt like a good opportunity to correct its field type and reduce our element's validation code a little bit. It could be this validation area was written before core had a numeric field type, or perhaps the original author was unaware of it. At any rate, this also enables client-side validation of the extension field, which is nice and something we've never had.

chrisolof → created an issue.

chrisolof → created an issue.

Looks good to me. Thanks Vaish!

Add Webform Element Values Limit under Enhancements section.

Merged latest 6.2.x changes into MR branch. Removed unused use statement. Added note about the default timezone that will be used to interpret the custom send-on date if not provided in the date string (so a source-code-dive isn't necessary to figure that out). Current state of the MR is attached, along with a screenshot of the changed field.

MR 425 opened. Status changed to needs-review. Proposed resolution and UI changes sections updated to match the resolution proposed in MR 425. Immutable diff (patch) attached here capturing the current state of this MR.