Account created on 8 February 2016, over 8 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States KurtTrowbridge

That's true for the From: value (who's sending the email), but I think the original request was about the To: value (who's receiving it)—and I actually just came looking for that this week as well. Ideally, I would think you'd be able to either enter a token that would fill in the value from the Webform submission, or you'd get to choose on a per-form basis which Email field from the form gets used.

🇺🇸United States KurtTrowbridge

I think this is related to this issue and its patch, but apologies if it's unrelated. I added this patch because I was having issues with a field that has multiple visibility conditions (e.g. Field 2 is visible if the answer to Field 1 was Option A or Option B). With the patch installed, that results in an error on line 64 ($trigger = key($triggerValue);) because of the or condition: the $triggers array that is looped through includes a non-array value, or, between the actual visibility conditions, so there's no array key to assign to the $trigger variable. I've attached a screenshot of what the $triggers variable contains. Not sure if it's as straightforward as confirming that $triggerValue is an array before continuing—I may try it and report back—or if that would cause an issue with the or condition not taking effect.

(Possibly of note: I have the patch installed atop version 2.0.10, not 2.0.x-dev, since the dev version is behind it and doesn't support version 6 of Webform.)

🇺🇸United States KurtTrowbridge

Making the suggestion to start with a single category, Content Editing Experience. Whenever I've used Entityqueue, it's been aimed at improving content editors' capabilities to feature or list content. Site Structure might also be worth considering, but I'll start with just the one.

🇺🇸United States KurtTrowbridge

Updating to a single category: Security.

🇺🇸United States KurtTrowbridge

Proposing just one category for this one: Security. Thought about Access Control since reCAPTCHA's often used on login forms, but it's a stretch, so keeping the fewer-is-better recommendation in mind.

🇺🇸United States KurtTrowbridge

We also ran into this problem, just as described in #16 (whenever a click event is triggered; you don't even have to click on interactive elements, though things like forms and accordions that require user interaction are naturally where it's most obvious), while running alpha3. Downgrading to alpha2 fixed the issue.

Based on the suggestion in #15, I provided an MR that tracks whether or not the menu is open, and adjusted the JavaScript in two problematic places so closeMenu() is only called if the menu isn't currently open. (I also snuck in a minor typo fix.) This appears to be working for us, but additional reviews/suggestions are much appreciated. Thanks!

If credited, please also credit my coworker joshcavindergmailcom , who helped me troubleshoot.

🇺🇸United States KurtTrowbridge

As mentioned in #4, header and footer text still don't appear if there are no results, despite a header and/or footer set to display even when there aren't any. I updated the MR to include that as well.

🇺🇸United States KurtTrowbridge

Addressed most of the feedback in the MR, hopefully correctly—the only one I think I left outstanding was the question about catching exceptions. Leaving marked as Needs work, and I'll see if JI_Gravityworks and I can touch base on that last one tomorrow (we work together).

🇺🇸United States KurtTrowbridge

This got in my way more this week, so I tried to figure out how to fix it. :)

I figured out that when the instagram_lite_media config value gets serialized before it's stored, null characters were being included in the output, and YAML won't parse that cleanly. By doing a preg_replace() that removes those, the config value is cleaned up, and the visual output of the Instagram feed on the page appears to be unaffected.

In addition to the provided merge request, I've attached a screenshot of the diff in my site's config after this patch has been applied, showing the removal of the null characters and the otherwise unaltered data (I guess it also turns from a multiline string to a single line...which is possibly fine? I don't display captions, but I do see line breaks in multiline captions get removed, which I could see being a problem for someone else).

This gets outside my typical wheelhouse a bit, so any additional reviews would be much appreciated. Thanks!

🇺🇸United States KurtTrowbridge

MR created with that plus a couple other small fixes (added missing punctuation at the ends of two sentences, and changed "grass roots" to "grassroots"). Thanks!

🇺🇸United States KurtTrowbridge

Hi! I followed the instructions in #7 while also excluding this module from the standard Drupal repository (see snippet below), and that installed the dependencies successfully and fixed the errors. Marking RTBC. Thanks!

    "repositories": [
        {
            "type": "composer",
            "url": "https://packages.drupal.org/8",
            "exclude": [
                "drupal/opigno_scorm"
            ]
        },
        ...
        {
            "type": "vcs",
            "url": "git@git.drupal.org:issue/opigno_scorm-3281986.git"
        }
    ],
🇺🇸United States KurtTrowbridge

Also confirmed that the supplied MR resolved the issue for my site using PHP 8.2 and the dev version of the module. Thank you!

🇺🇸United States KurtTrowbridge

One more confirmation that the patch in #9 is working on a site with Drupal 10.2, PHP 8.2, and the dev version of this module. Going to mark this as RTBC. Thanks!

🇺🇸United States KurtTrowbridge

Hello! I tested this, first confirming that I saw the PHPCS errors mentioned when the patch was not yet applied. After applying the MR as a patch, the PHPCS errors are no longer present. Marking RTBC as a result.

(That said, I should note that line 60 in the .theme file—the comment starting with "Calculate the minimum and maximum"—warns me, both before and after, that it's lengthier than the 80-character limit in Drupal's standards. Not sure why it doesn't appear in your testing, so I didn't flip back to Needs Work in case it's just something I'm doing differently on my end.)

🇺🇸United States KurtTrowbridge

I've been poking at this a little bit tonight, and I think it's an issue specific to the way the Claro theme (and probably others based on it, like Gin) handles focus indicators—the typical green outline visible when focusing on an element in Claro is actually a box-shadow, and the outline is transparent (I think because of something having to do with Windows High Contrast Mode). I'm not positive why the box-shadow doesn't show up, but assume it has something to do with the <a> element being within an <svg> element.

If I switch to another admin theme (I switched to Umami; it's meant to be a front-end theme, but still illustrated what I needed), I do see focus indicators when tabbing through the diagram elements.

Anyway, a possible solution could be to target .mermaid svg a:focus and apply an outline-color there. I wonder if that might be more suitable in the Mermaid Diagram Field module, so feel free to move this there if you'd like and I can provide a new MR, but I'll include one that adds the outline-color with a value of var(--color-focus) for themes like Claro that provide that CSS variable, and a fallback to currentColor for those that don't.

🇺🇸United States KurtTrowbridge

(Working on this at FLDC 2024.) There was one subfolder, tests/src/Functional/Update, that had a file that was missing the declare statement. I added it to the existing MR for this issue. Now marking Needs Review.

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

Just missing a preposition ("to retrieve information"), so slight tweak below:

Provides a file metadata manager service and API to allow developers to retrieve information stored in files, like EXIF photo information, TrueType font information, and others.

(Character count: 177)

🇺🇸United States KurtTrowbridge

Very slight tweak, assuming "ex.g." was a typo—I like the latest suggestion otherwise:

Allows users to add additional filters (facets) to search results, to further refine search results by some criteria (e.g. Amazon's search interface).

Character count: 150

(Leaving as Needs Review since I changed it.)

🇺🇸United States KurtTrowbridge

Hello! I'm looking at this during Florida DrupalCamp 2024, and freshly installed Project Browser with the MR applied. The placeholder text appears as expected, both in the visual output of the search field and in the DOM as a placeholder attribute. Screenshot attached of the visual output.

Going to mark this as RTBC. Thanks!

🇺🇸United States KurtTrowbridge

I moved the suggested README contents in #2 into an actual file on a new branch, then cleaned it up based on the recommended format . MR is ready for review.

🇺🇸United States KurtTrowbridge

(Working on this at FLDC 2024.) I'm not seeing the error when running PHPCS in the first place, so a review would definitely be appreciated, but the fix seems straightforward enough. PHPCS did not find any errors following my change either.

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

I know this module is obsolete now, and we'll be getting rid of it on the site on which I tested this once it's upgraded to Drupal 10, but for any other fellow stragglers on Drupal 9, the patch in #2 solves the issue.

🇺🇸United States KurtTrowbridge

This also resolved issues with grouped proximity filters on a site of ours. Since others have said the same above, I'm going to mark this as RTBC. Thank you!

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

Thanks for catching that! That does appear to have done the trick, so marking as RTBC.

🇺🇸United States KurtTrowbridge

That did the trick, thanks! Marking RTBC. Good to know about the upcoming core improvement too.

🇺🇸United States KurtTrowbridge

Here's patch #20 again, this time with the PHP CodeSniffer issues resolved.

🇺🇸United States KurtTrowbridge

Suggesting the following description:

Adds filters to the top of the Permissions list, reducing it to only include permissions related to the selected user roles and/or modules.

Character count: 139.

🇺🇸United States KurtTrowbridge

Thanks for the quick reply! I had another chance to look further into this tonight, and unfortunately still don't have an answer. What I tried:

  • Switching the datalayer_page_bottom() hook to datalayer_page_top()
  • Uninstalling any other modules that implemented hook_page_bottom() (AMP, Mailchimp, Tour)
  • Removing my custom theme's html.html.twig template (even though it did have {{ page_bottom }} included)

I've included my datalayer.settings.yml contents below.

_core:
  default_config_hash: N67AkcKPlikWJbwxABv6MSMpGuXe36qWZfH88pEzeqc
add_page_meta: true
output_terms: true
output_fields: true
remove_from_admin_routes: false
lib_helper: false
entity_meta:
  created: '0'
  langcode: '0'
  name: '0'
  status: '0'
  uid: '0'
  uuid: '0'
  vid: '0'
enable_ia: false
ia_depth: 3
ia_category_primary: primaryCategory
ia_category_sub: subCategory
vocabs:
  tags: tags
  column: '0'
expose_user_details: ''
expose_user_details_roles:
  authenticated: '0'
  content_editor: '0'
  social_media_editor: '0'
  administrator: '0'
current_user_meta:
  name: '0'
  mail: '0'
  roles: '0'
  created: '0'
  access: '0'
group: false
expose_user_details_fields: false
entity_title: entityTitle
entity_type: entityType
entity_bundle: entityBundle
entity_identifier: entityId
group_label: groupLabel
drupal_language: drupalLanguage
drupal_country: drupalCountry
site_name: siteName
key_replacements:
  - ''

And it case it's relevant, the hook_datalayer_alter() implementation through which I handle multivalue fields and a couple custom additions:

/**
 * Implements hook_datalayer_alter().
 */
function bridge_utilities_datalayer_alter(&$data_layer) {
  // Modify data layer for Tags.
  if (isset($data_layer['entityTaxonomy']['tags'])) {
    $data_layer['articleTags'] = implode('|', $data_layer['entityTaxonomy']['tags']);
  }

  // Modify data layer for Authors.
  if (isset($data_layer['field_author'])) {
    $authors = $data_layer['field_author']['value'];
    $author_names = [];
    foreach ($authors as $author) {
      if (!empty($author['label'])) {
        $author_names[] = $author['label'];
      }
    }
    $data_layer['articleAuthors'] = implode('|', $author_names);
  }

  // Modify data layer for publish date.
  if (isset($data_layer['entityCreated'])) {
    $data_layer['articleDate'] = date('Y-m-d', $data_layer['entityCreated']);
  }

  // Modify data layer for donation confirmation order ID and revenue.
  if (\Drupal::request()->get('uid') !== NULL) {
    $data_layer['order_id'] = \Drupal::request()->get('uid');
  }
  if (\Drupal::request()->get('amount') !== NULL) {
    $data_layer['revenue'] = \Drupal::request()->get('amount');
  }
}

Thanks again!

🇺🇸United States KurtTrowbridge

I added a new fork, 3392147-allowed-ips, with the patch provided in #5 plus some inclusive language changes. (Sorry if I didn't do that right—the amount that the existing MR had changed in unrelated files threw me off, so I was hesitant to try to build on top of it.) I also did a little bit of formatting cleanup, but that probably wasn't exhaustive.

One thing I'm confused by is that when I look at the allowlist, the IP addresses won't show up in the column (see attached screenshot)—something seems off with how the ID and Allowlisted IP properties are defined and saved, but I haven't yet figured out what. Additionally, it looks like the IP address is intended to be saved as the entity ID; this works for IPv4 addresses, but causes a fatal error ("Invalid character in Config object name") when allowing IPv6 addresses due to the colons (:) in the address.

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

I set up Mercury Editor and the Inline Editor submodule on a test site and confirmed I initially could not edit a text Paragraph inline. Then I patched it with MR #17, and confirmed that inline editing was possible again. Thanks!

🇺🇸United States KurtTrowbridge

I've updated the merge request based on the provided feedback—should be ready for review again. Thanks!

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

Confirmed that MR #2 also works (alongside my earlier MR on 🐛 Entity queries must explicitly set whether the query should be access checked or not Needs review , which I previously forgot to mention I also have in place to explicitly set access checks on a few queries). Leaving as RTBC!

🇺🇸United States KurtTrowbridge

The patch in #21 did not apply for me using Composer, I think because of the problem described in 🐛 Packaging info from .info.yml often creates conflicts when patching Active . Merge request #7, however, does successfully apply, so leaving this as RTBC. The only difference I see between the two is that drupal/image is not removed as a dependency in MR #7, but I think its removal was unintentional anyway based on the patches and comments in #16 and #19 here, so that seems fine to me.

🇺🇸United States KurtTrowbridge

Hi! I ran into this today too on a site running Drupal 9.5.11. I was able to get the newly added handler to open its configuration form after making some adjustments, matching a couple other Webform handlers I referenced. After that, I hit some issues related to the Comparison Operator field due to how the title was formatted, so I provided an adjustment that moves the additional information for that field into a description. Lastly, I had an issue saving the configuration form because it tried to validate the comparison operator based on the value of $values['compare']['operator'], when the variable appears like it should be $values['operator']–only the left and right values are nested under $values['compare'].

Hopefully that all makes sense. I confirmed that the comparison was working for me following those adjustments, and confirmed that the file meets Drupal's code standards (one other adjustment needed for that was to put the use statements in alphabetical order). Everything I've mentioned is in the provided merge request, but I'd definitely appreciate a review—dependency injection is not my strong suit, and a couple of the fixes were a little unexpected to me.

Thanks!

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

First, I think this may be a duplicate of #3303510 📌 File metadata manager - Add a short description Needs review .

I like Chris's suggestion in #9 there, and just tweaked it slightly to add a missing preposition:

Provides a file metadata manager service and API to allow developers to retrieve information stored in files, like EXIF photo information, TrueType font information, and others.

🇺🇸United States KurtTrowbridge

Went back and forth on a few ideas, but put this suggestion into the issue summary:

Provides a text input that allows users to type a keyboard shortcut and enter the name of a requested Drupal admin page. Offers page suggestions based on the entered keywords.

My primary thought is that we should avoid specifying the Alt+D keyboard shortcut in the description, because (as is detailed in the module's full description) that's not accurate in all cases (and, thinking about future-proofing it, perhaps unnecessarily, I know there are some feature requests in the module's issue queue that would allow sites to customize that shortcut). Character count on my suggestion is 175.

🇺🇸United States KurtTrowbridge

I like and agree with Emma's feedback in #7. I tweaked that slightly based on the language I've seen used in other descriptions, and added that to the issue summary. Character count still looks good, now at 147.

🇺🇸United States KurtTrowbridge

Agreed with the most recent edits. Character count is 134, so all good there. Marking as RTBC!

🇺🇸United States KurtTrowbridge

This one looks good to me. Character count is 152. Marking as RTBC!

🇺🇸United States KurtTrowbridge

Merge request provided and ready for review. Attaching a screenshot of the revised fieldset with the MR applied.

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

Screenshot attached, but the buttons look right again with the MR applied! Marking as RTBC, though I'm not sure if the automated test failure on the MR is a problem. (When I ran the code check script locally, ESLint didn't find any issues.)

🇺🇸United States KurtTrowbridge

Tweaked slightly, mostly to capitalize Views, but also while thinking that "This module" and "Drupal" might be unnecessary to include; just a thought. I agree with the previous comments about including the relevant file extensions, and confirmed that the three included are the ones it supports by default (the module description includes more, but they're merely on the roadmap, per the code's README).

🇺🇸United States KurtTrowbridge

I like what's proposed; I just tweaked it slightly because the use of "content types" gave me pause since that has its own unrelated meaning in Drupal. Character count is now 189.

🇺🇸United States KurtTrowbridge

Updated the issue summary, suggesting a couple changes to the description. First, I removed the repetition of the module name, something I've seen done in other proposed descriptions, and the name was inaccurate here anyway. Second, I removed mentions of the File Entity and Media Entity modules; while they're noted in the module's description, I'm not sure how directly relevant they are these days to the module's utility, especially given that Media Entity has since been replaced by the core Media module. Definitely not opposed to further adjustments, though.

🇺🇸United States KurtTrowbridge

Hmm, I'm unable to reproduce this one (I tested in both Chrome and Safari on macOS Ventura). Screen recording attached of me (in Chrome) tabbing through the filters based on the described steps to reproduce—I see a focus indicator on the close button for each filter as I tab through, including after removing or adding filters. Am I misunderstanding anything, or is this all set already?

🇺🇸United States KurtTrowbridge

I did a little more digging here, and Safari has an issue styling hover/focus states on checkboxes and radio buttons. The recommendation to fix it usually seems to be resetting checkbox styles with appearance: none; and -webkit-appearance: none, alongside other CSS to restyle the inputs. In testing a few themes, the Olivero and Adminimal themes both do that for all radio/checkbox inputs, and as such, hover/focus states on checkboxes in the sidebar look as expected in Safari while using either of those as the admin theme; Claro and Gin do not have that CSS and end up with no applied hover/focus styles.

I noticed #3300262 📌 Adjust the checkbox design for the categories sidebar to the Drupal Design system Postponed already brought up the need for the checkbox styles to change to match the Drupal design system, including the impact it would have on hover styles, and that's postponed due to larger CSS refactoring work in #3285889 🌱 [meta] Svelte app themability Needs work . Not sure if the former makes this a duplicate or if there's more we should do in this issue.

🇺🇸United States KurtTrowbridge

Confirmed the described behavior on Safari: if I try to tab through the page, I only get the keyword search, the sort filter, and the tabpanel with the module list.

However, I think that may be an issue with Safari browser settings, rather than anything that needs to be changed in the code here: there's seemingly a years-long issue in Safari where you have to adjust its settings (Safari > Settings > Advanced > check "Press Tab to highlight each item on a webpage") in order for tabbing to function as expected. Here's an example blog post about it: https://www.seanmcp.com/articles/tab-focus-not-working-in-safari/ The Stack Overflow post it links to goes back to 2009.

Once I enable that setting, I can tab through the page as expected. There may be an issue with the checkboxes along the side, since I don't see a focus state applied to those (only in Safari; I do in Chrome and Firefox) as I tab through, but everything else looks fine to me.

🇺🇸United States KurtTrowbridge

Tweaking #5 a bit since the character length was beyond the 200-character maximum, pulling out the repetition of the module name in the description, and also trying to incorporate some of the suggestions in #2. New suggestion:

Allows module and theme developers to provide detailed documentation for site builders to understand its features.

🇺🇸United States KurtTrowbridge

Liking #9; just suggesting another slight tweak to possibly remove a bit of redundancy:

Simplifies module search for site builders by providing tabs and keyword search.

Character count: 80

🇺🇸United States KurtTrowbridge

I like the description in #5—just trying to bring the reading level down further (without much success, so feel free to continue to tweak):
"Allows administrators to set password rules, like minimum character count or required characters, to ensure strong password creation."

(also adding to the issue summary)

🇺🇸United States KurtTrowbridge

New 512x512 logo with transparent background provided.

🇺🇸United States KurtTrowbridge

Working on this one while at DrupalCon Lille.

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

Bonjour from Lille! I adjusted the top and middle regions to use aria-labelledby attributes. MR created. :)

🇺🇸United States KurtTrowbridge

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

🇺🇸United States KurtTrowbridge

The merge request provided worked for me, allowing me to uninstall the module. Thanks!

🇺🇸United States KurtTrowbridge

In case it helps, I had this problem as well, but resolved it by ensuring my node template had the {{ content }} variable in it. (I had been more specific, with variables like {{ content.body }}, but I swapped it out in favor of {{ content|without('fields_I_did_not_want') }}.)

🇺🇸United States KurtTrowbridge

Whoops—trying again with PHPCS fixes in place.

🇺🇸United States KurtTrowbridge

This has been working well for me for a while on 1.0, but I had to reroll against the newest version of the module. Patch attached.

🇺🇸United States KurtTrowbridge

The Drupal Rector patch applies successfully. Marking as RTBC. Thanks!

🇺🇸United States KurtTrowbridge

Confirming that the Drupal Rector patch in #4 is working here. Thanks!

🇺🇸United States KurtTrowbridge

Also confirming that the Drupal Rector patch in #2 works. Thanks!

🇺🇸United States KurtTrowbridge

Also confirming that the patch in #4 works. Thanks!

🇺🇸United States KurtTrowbridge

#7 is working for me as well. Marking as RTBC. Thanks!

🇺🇸United States KurtTrowbridge

No disagreement from me! Updating the issue summary and flipping back to Needs Review.

Production build 0.69.0 2024