New York
Account created on 23 October 2009, almost 15 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States FatherShawn New York

Please re-open if this work becomes active again

🇺🇸United States FatherShawn New York
🇺🇸United States FatherShawn New York
🇺🇸United States FatherShawn New York
🇺🇸United States FatherShawn New York

Thank you @dww for Option D. The scope is definitely noise in contrib as it would be the same for every commit in the project. Even in core, anything not in a module might be scope (core)?

+1 for Option D.

🇺🇸United States FatherShawn New York

fathershawn created an issue.

🇺🇸United States FatherShawn New York

Postponed until Entity Share has at least a dev version for D11.

🇺🇸United States FatherShawn New York

Fixed in 1.2.0-beta1

🇺🇸United States FatherShawn New York

Some additional adjustment to FormBuilder is needed, and I could use guidance as to where. The POC is on admin/config/development/configuration/single/export

Here's the problem sequence:

  1. Load the form
  2. Change the type
  3. Select a name
  4. Change the type

At this point the form class is called to build the form, but this but doesn't run:
FormBuilder.php:1294

      // Detect if the element triggered the submission via Ajax.
      if ($this->elementTriggeredScriptedSubmission($element, $form_state)) {
        $form_state->setTriggeringElement($element);
      }

so the name is not reset.
Then the code above is executed but the form is not rebuilt.

I feel like we therefore either need to set a rebuild if our request is htmx but I'm not sure the correct place to do that in form builder.

Here is a diff with 11.x of the current state of the work.

🇺🇸United States FatherShawn New York

From the core issue Remove usages of MigrateSkipProcessException from core process plugins

🇺🇸United States FatherShawn New York

Omitting the validation function so I can address 📌 Rename the payment method add form class Active

🇺🇸United States FatherShawn New York

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

🇺🇸United States FatherShawn New York

A closer look and I have a better answer. There is a small but important difference between the templates.

The template at commerce/modules/order/templates/commerce-order-receipt.html.twig has this block:

{% block payment_method %}
  {{ payment_method }}
{% endblock %}

And the template in this module has:

{% block payment_method %}
  {{ payment_method }}
  {{ payment_instructions }}
{% endblock %}

The payment instruction info is preprocessed in commerce_purchase_order_preprocess_commerce_order_receipt. I've changed the template in this module to a clear twig block override. Please would this have saved you time?

🇺🇸United States FatherShawn New York

@jsacksick Please review and comment on the MR above.

🇺🇸United States FatherShawn New York

This is a bug. The feature was contributed and I missed this in the review. Further, manual testing shows that the Purchase order output is not useful or accurate when a file is used. Fixing that too.

The core of this issue is that the logic about what information is needed in PurchaseOrderGateway::createPaymentMethod is not correct.

🇺🇸United States FatherShawn New York

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

🇺🇸United States FatherShawn New York

Commerce 3.x is not released but this is ready for when it is.

🇺🇸United States FatherShawn New York

fathershawn changed the visibility of the branch 3429385-automated-drupal-11 to hidden.

🇺🇸United States FatherShawn New York

Also has deprecations and changes in upstream text expected

🇺🇸United States FatherShawn New York

This will be addressed in the D11 compatible version

🇺🇸United States FatherShawn New York

I've sent a note to @Sutharsan via his contact form asking him to clarify the status.

🇺🇸United States FatherShawn New York

@rodrigoaguilera I'm fond of this module and about to use it in a project. Have you contacted the existing maintainers via their contact form and did you receive any response?

🇺🇸United States FatherShawn New York

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

🇺🇸United States FatherShawn New York

That's great! Thanks for testing. Yes - you use the standard core block, HTMX Loader, which is placed and configured normally. When configuring it you can choose what triggers along with some other htmx attributes. The default swap behavior is that HTMX block replaces the HTMX Loader, but you can configure that too.

The HTMX block feature definitely has utility: using load as the event one could conditionally lazy load a block as one one use case. However, it principally demonstrates how a developer could use HTMX to create dynamic elements.

🇺🇸United States FatherShawn New York

@bbu23 I've tagged version 1.2.2-alpha1 with a refactor to address this issue. Would you be willing to see if it fixes your failure?

🇺🇸United States FatherShawn New York

That's super helpful. Removing the header makes it work because the header size is huge. It looks like there is markup stored in drupalSettings. We may have to revert or adjust a design choice.

HTMX allows us to trigger an event via header and include data for that event. So rather than appending JSON into a script tag with new ajax page state at the bottom of the DOM and then spending time to find, load and parse it as core ajax does, we included the data in this way.

It looks like Magento has had a similar problem with cache tags:

🇺🇸United States FatherShawn New York

Thanks for continuing to examine this on your end for clues. Switching the status of this issue to "needs info" is the right idea too. Because HTMX is designed to work with standard HTML responses, you can copy the failing request from your network tab and load it directly as your admin user. In a working setup, the dialog tag will be present and populated. I've added a screenshot. In your case, perhaps you can get better insight into the 500 error working with the request directly.

🇺🇸United States FatherShawn New York

I did a fresh site build of a plain 10.2.7 site and changed my ddev setup to php 8.2 (8.2.20). With HTMX as the only contrib module, I could not reproduce the error. I've attached a video . I don't know what to suggest since your logs are empty. If this is in a built up site, could there be a conflict with another module?

🇺🇸United States FatherShawn New York

Thanks for the report. I'll verify and get a fix out or post back swiftly as it worked the last time I used it!

🇺🇸United States FatherShawn New York

Thank you for your encouragement and your thanks. It's a real learning adventure exploring this new tech!!

Thanks for the merge request and I do understand your motivation, but that's not open to us until the coding standards change as they require only the one line on inherted methods.

You might choose to participate in the conversation at #1994890: Allow {@inheritdoc} and additional documentation if you would like to influence that standard.

🇺🇸United States FatherShawn New York

False alarm - I was walking a colleague through the functional flow on my local demo and I must have rebuilt my demo site at some past time - my cloudflare config was empty.

🇺🇸United States FatherShawn New York

Investigating an error using the most recent version of 10.3: "A path was passed when a fully qualified domain was expected."

🇺🇸United States FatherShawn New York

Well, #20 📌 [POC] Implementing some components of the Ajax system using HTMX Active is not the right solution for where
to merge in '#htmx' because of the nature of template_preprocess_HOOK such as this in form.inc. Probably why existing ajax is doing it's work at the element pre-process level.

/**
 * Prepares variables for select element templates.
 *
 * Default template: select.html.twig.
 *
 * It is possible to group options together; to do this, change the format of
 * the #options property to an associative array in which the keys are group
 * labels, and the values are associative arrays in the normal #options format.
 *
 * @param $variables
 *   An associative array containing:
 *   - element: An associative array containing the properties of the element.
 *     Properties used: #title, #value, #options, #description, #extra,
 *     #multiple, #required, #name, #attributes, #size, #sort_options,
 *     #sort_start.
 */
function template_preprocess_select(&$variables) {
  $element = $variables['element'];
  Element::setAttributes($element, ['id', 'name', 'size']);
  RenderElementBase::setAttributes($element, ['form-select']);

  $variables['attributes'] = $element['#attributes'];
  $variables['options'] = form_select_options($element);
}
🇺🇸United States FatherShawn New York

As I look more deeply at implementing the idea from @fgm in #17, it seems to me that the interface should be much smaller than the public methods, otherwise we're just moving everything in Attribute to various traits or something as there is only one protected method and it has the logic for creating values before storage. Many of the other existing public methods implement other Interfaces for Attribute.

I looked through all the places we currently test for instanceof Attribute and I propose these methods on the interface:

  • ::hasAttribute
  • ::toArray
  • ::merge

Because in templates we want the full suite of Attribute methods, I'd leave these instanceof conditionals as they are in AttributeHelper

    // If at least one collections is an Attribute object, merge through
    // Attribute::merge.
    $merge_a = $a instanceof Attribute ? $a : new Attribute($a);
    $merge_b = $b instanceof Attribute ? $b : new Attribute($b);
    $merge_a->merge($merge_b);
   
    return $a instanceof Attribute ? $merge_a : $merge_a->toArray();
🇺🇸United States FatherShawn New York

It looks to me like the Attribute magic happens in template_preprocess found in core/includes/theme.inc. If I add a #htmx key in a render array, and then add

if (isset($variables[$key]['#htmx'])) {
  $variables['attributes'] = AttributeHelper::mergeCollections($variables['attributes'], $variables[$key]['#htmx']);
}

right after the existing #attributes merge, then a breakpoint at the assignment here trips.

🇺🇸United States FatherShawn New York

In addition to all check green, manual test is good. Merging

🇺🇸United States FatherShawn New York

I decided to implement the inheritance version of this strategy in the contrib module so that people can easily try it out. The MR is pending with the extended class and a documentation page with some usage examples for that implementation.

This issue's implementation would use composition, and have a dedicated key, taking care of merging the attributes in processing. That said, feedback welcome over in contrib to refine any of this before we stand it up here in Core.

🇺🇸United States FatherShawn New York

All quality checks are now green

🇺🇸United States FatherShawn New York
🇺🇸United States FatherShawn New York

Yes, that's the plan. I needed to test or someone to test the replacement first so thanks for the report

🇺🇸United States FatherShawn New York

This is an opportune time to evaluate retiring this module. The team at CKSource (developers of CKEditor) have released CKEditor 5 Plugin Pack which includes the plugin provided by this module.

Please evaluate their officially supported module and add a note back if ckeditor_find is still needed.

🇺🇸United States FatherShawn New York

Thank you @fgm for reminding me that we are working on core! I was surprised there wasn’t an interface but I’m so used to working in contrib, I thought inheritance was what we had. We totally need an interface here.

Production build 0.71.5 2024