🇺🇸United States @jrockowitz

jrockowitz → created an issue.

jrockowitz → created an issue.

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

Okay my mistake the issue is with the media_contextual_crop_iwc_adapter

jrockowitz → created an issue.

jrockowitz → created an issue.

jrockowitz → created an issue.

I updated the project page based on your suggestions.

I had a few minutes to test this patch.

Below are my basic steps to review

• Check out the MR
• Clear cache
• Add custom field to page content type
• Create a link field using default values
• Create a link field that only shows URL, placeholder, attributes, etc...
• Confirmed default link field worked as expected
• Confirmed customize link field worked as expected

The most immediate minor issue I found was adding a new link field logged the below warning multiple times.

Deprecated function: Creation of dynamic property Drupal\custom_field\Plugin\DataType\CustomFieldLink::$value is deprecated in Drupal\Core\TypedData\TypedData->setValue() (line 102 of core/lib/Drupal/Core/TypedData/TypedData.php).

jrockowitz → created an issue.

Done!

https://www.drupal.org/project/eca_report →

I will promote my sandbox to a dedicated ECA report module, allowing people to contribute to it there. On the project page, I will reference this ticket.

If the consensus is that this functionality will eventually be integrated into the Modeler API and ECA (core), I could promote the sandbox to a project with the 'eca_report' namespace. The only downside of that decision is that we are establishing an 'eca_report' namespace, which might be around for only a few years.

Based on #11, the new modeler API can provide a plugin usage report. The ECA Report module adds some additional columns to the reports based on the plugin type.

It might make the most sense to postpone this and leave the code in the sandbox module AS-IS.

The \Drupal\eca_report\Controller\EcaReportBaseController::getUsedIn method is relatively basic and could be reworked to support the Modeler API. At the same time, in 3.x, the ECA entity is not being changed, so we might not have to tweak any code.

My hope is that we select an MVP solution that can be easily iteratively improved without requiring additional work for the ECA maintainers.

I do see that eca_ui.module is going to change significantly with 3.x, and maybe adding this report to that module is not the best approach.

I appreciate the immediate feedback.

For #2, similar to the Views plugin report, we should only list enabled plugins.

For #3, also similar to the Views plugin report, I added a 'Used in' to track an ECA plugin's usage.

jrockowitz → created an issue.

jrockowitz → created an issue.

jrockowitz → created an issue.

jrockowitz → created an issue.

jrockowitz → created an issue.

@see 🐛 Remove deprecated SDC module dependency Active

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

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

We could implement the work-around inside the webform module if required.

The patch removes the expanded attribute which I think improves the accessiblity for screen readers.

Even though a machine/automated review says the tag + attributes is invalid, it does not mean we should remove the attribute to pass an automated test.

I am not sure this is resolvable. Changing the tooltip's HTML tag will cause visual regression on the sites targeting the SPAN tags. You might have to use the patch AS-IS.

The challenge will be adding Ajax callbacks to the widget.

@damienmckenna I am hesitant to add new features, but maybe this is simple to implement, and the variant dropdown is only visible if the webform is using variants.

The webform nodes 'References' kind of allows people to build nodes for different variants.

→

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

jrockowitz → changed the visibility of the branch 3517069-accessibility-issue-tabs to hidden.

This is most likely the future direction for external libraries.

✨ Create a mirror for external library dependencies for composer support Active

6.3.0-beta2

Please create an MR and lets see if the test pass.

I am not sure we can make this type of access control change without impact existing expectations.

People should weigh-in if this enhancement is useful.

jrockowitz → created an issue.

jrockowitz → created an issue.

I think the SchemaDotOrgFieldHelper does improve things.

The short answer is people have to upgrade to resolve this issue

I reverted the commit.

@BramDriesen My jaw dropped when I logged back in. I was about to re-roll the MR when I saw that you had already done it. Thank you!!!

We probably need to rebase this because of 📌 Convert to OOP Hooks Active

Your persistence has paid off. I am at DrupalCon is time to merge this. Thank you!!!

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

While it is a big patch, this is the same process used for core to convert, and we can spot check that the conversions are accurate. Since this is rector it's really just copying the hooks to a hook class an method.

Webform also has more tests than most modules so we can have more confidence than most modules would have that this won't introduce systemic issues.

I completely agree with you that this should not introduce systemic issues and merging it would nudge more contrib modules to use OOP hooks.

I think 🐛 Checkboxes/Radios with description display set to "before" renders description in #prefix Active might have addressed this issue.

I have to agree that this is a release blocker

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

Since we are changing the $form array, this should be only committed to 6.3.x

Here is the change record. With tests passing, I am fine with merging this.

There are security implications for allowing someone to specify the submission owner via query parameter.

ChatGPT suggested the below approach which is a valid solution

/**
 * Implements hook_webform_submission_presave().
 *
 * Allow setting the webform submission owner from a query string parameter.
 */
function MYMODULE_webform_submission_presave(WebformSubmissionInterface $webform_submission) {
  // Check if a query parameter 'submission_owner' exists.
  $owner_uid = \Drupal::request()->query->get('submission_owner');
  
  if ($owner_uid && is_numeric($owner_uid)) {
    // Ensure the user ID exists in the system.
    $user = \Drupal\user\Entity\User::load($owner_uid);

    if ($user) {
      // Set the submission's owner to the user ID from the query string.
      $webform_submission->setOwnerId($owner_uid);
    }
  }
}

Patch seems very safe.

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

I think we need to fork the webform bootstrap 3 module into a dedicated project and deprecate the code in the webform module.

In a form alter hook, you can change the $form['progress'] weight or alter the $form array.

ChatGPT suggested the below approach, which is completely valid and the simplest solution

/**
 * Implements hook_form_alter().
 */
function mymodule_form_alter(array &$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {
  // Check if the form ID matches the webform submission form.
  if ($form_id == 'webform_submission_form') {
    // Save the progress element into a temporary variable.
    $progress_element = $form['progress'];
    // Remove the progress element.
    unset($form['progress']);
    // Add the progress element to the end of the form.
    $form['progress'] = $progress_element;
  }
}

Please review the MR based on #17 which tries to keep the changes as simple as possible.

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

I am for any patch that simplifies the JS code and is safe to commit.

Core at some point reversed https://www.drupal.org/node/1570578 → and use strict is no longer included in core JS.

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

Let's update the URL field to support 2048 characters @see https://serpstat.com/blog/how-long-should-be-the-page-url-length-for-seo/

Let's update the URL field to support 2048 characters @see https://serpstat.com/blog/how-long-should-be-the-page-url-length-for-seo/

Let's update the URL field to support 2048 characters @see https://serpstat.com/blog/how-long-should-be-the-page-url-length-for-seo/

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

I think the current OOTB approach is the use something like IMCE → to upload a file to the public file directory and attach the file via the Attachment URL element.

→

If we were going to support attachments via file upload I would opt for Attachment File element, which could be maintained in a dedicated contributed module. Does this approach make sense?

Make sense that a mailer application needs the body to be a string.

Adding a new property to Name would require a new 6.4.x version of Webform to be created.

This improvement should be handled in a dedicated contrib module. You can also use https://www.drupal.org/project/webform_composite → to create this variation of a name input.

I tested the fix in Chrome and Firefox and it looks good.

MRs are welcome to help resolve this

Fixing in 6.2.x and 6.3.x

We have been using this patch on production and it seem sreasonable that we want to ensure the webform_image_select.element library loads after webform.filter.js

The tweak looks fine to me, and enough people have reviewed this

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