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

Merge Requests

More

Recent comments

🇺🇸United States fathershawn New York

I still need to deal with Messages and Redirects, but this approach is simpler and over a 10 request sample results in a mean time of Load of 178.6 ms compared to 185.4 ms for the existing 11.x branch.

🇺🇸United States fathershawn New York

Here's an MR into @nod_'s branch moving the bigPipe command JS into bigPipe scope. It looks like the test setup scripts don't like MR's into a feature branch. The tests generally pass locally - 2 failures involving noJS exceptions. Not sure if that's a false negative as all of these changes are in JS.

If this is an acceptable direction then I think the next level of refactoring is to create the needed bigPipe Command classes to replace the AjaxCommands and update BigPipe.php to work without the Ajax API classes.

🇺🇸United States fathershawn New York

fathershawn changed the visibility of the branch 3526267-bigpipe-with-htmx to hidden.

🇺🇸United States fathershawn New York

We need this only if if there is an existing capability we cannot implement using the tools provided by htmx - let's keep it a placeholder.

🇺🇸United States fathershawn New York

All green is super - nice work! This is direction really simplifies the changes in big_pipe.js and that’s a real plus.

Because bigPipe as we have it re-uses existing javascript from the ajax system, to remove the ajax system we need to replace that.
@nod_ had this list in a Slack discussion:

We need the equivalent of those 3 commands to implement bigpipe

  • use Drupal\Core\Ajax\MessageCommand;
  • use Drupal\Core\Ajax\RedirectCommand;
  • use Drupal\Core\Ajax\ReplaceCommand;
  • Add_css/add_js and the settings but that’s mostly available already

I think we need to identify the pieces of this list that htmx is not going to do, and implement those in general scope. Then the items that htmx does just fine in an ajax context, we need to have as helper methods in big_pipe.js and leverage the htmx api where we can.

I'm going to take them in order.

MessageCommand
In the ajax context, I would have implimented this in HTMX using a header and a custom event handler that calls our Drupal.Message function. This is actually the example in the trigger headers documentation.

HX-Trigger: {"showMessage":{"level" : "info", "message" : "Here Is A Message"}}

One thing that Drupal.Message does client-side is determine the selector for the message container. If there is a way to do that server-side then this could also be implemented as an extra out-of-band swap on the response without any additional javascript.

We do need a helper method for this that we can use in big_pipe though.

RedirectCommand
HTMX has both the HX-Redirect and HX-Location response headers. We don't have a response so we need a function for that in bigPipe.

ReplaceCommand
We also just need the bigPipe use case for bigPipe. This is one of the core competencies of htmx in an ajax request context.

addCSS and addJS
In the ajax context using htmx, these feel like out of band swaps as well.

🇺🇸United States fathershawn New York

We agreed in Slack to pause on this builder while we work on 📌 Refactor BigPipe to use HTMX Active using headers that are "hand-rolled."

🇺🇸United States fathershawn New York

We agreed in Slack to pause on this builder while we work on 📌 Refactor BigPipe to use HTMX Active using attributes that are "hand-rolled."

🇺🇸United States fathershawn New York

What ever we implement, if there is configuration we should collect in via \Drupal\dynamic_yield\Form\FeedSettingsForm and store it in the general settings.

🇺🇸United States fathershawn New York

We already to not load DY on admin routes. The presenting issue is that some pages often use the admin theme, such as entity edit forms. This seemed like a simple way to exclude certain path patterns.

We could programmatically detect the active theme, get then name of the admin theme and compare to exclude DY.

🇺🇸United States fathershawn New York

Thank you @catch. I wanted to layout the concerns expressed to me, and I'm comfortable with our original course. I think it is cleaner to create and transition to a parallel system. The HtmxInterface that I propose in the issue summary offers two paths for a developer.

A developer does not need to know anything about how HTMX works. Choosing from a defined set of operations, such as Replace or Insert, the developer adds the required data to the operation and inserts it into the Htmx object.

Alternatively, a developer can dive completely into the world of HTMX using the attribute and header subsystem objects that are part of the Htmx object. I guess there is also a third path which is to create their own implementation of HtmxOperationInterface to package these into something reusable.

This offers both simplicity and power in something that I think is straightforward to manage. Creating all the operations will be our final task in the initiative. I'm confident that we can offer a migration guide that maps ajax commands to htmx operations.

🇺🇸United States fathershawn New York

Tests revealed some issues

🇺🇸United States fathershawn New York

That's the right idea :). When I have time I'll inspect the render array there.

🇺🇸United States fathershawn New York

And in fact I did that in \Drupal\htmx\Render\HtmxBlockView::build. So now to figure out why is didn't do that for the caching

🇺🇸United States fathershawn New York

fathershawn created an issue.

🇺🇸United States fathershawn New York

It sounds to me like what is needed is that the dedicated render controller for the HTMX block needs to use the context of the requesting page.

🇺🇸United States fathershawn New York

Looks like there are over 60 projects on d.o using this approach now.

🇺🇸United States fathershawn New York

This module uses the https://github.com/ddev/ddev-drupal-contrib approach for module development. This makes the module easy to maintain. Drupal CMS also has a committed `.ddev` directory although their setup isn't for a contrib module: https://git.drupalcode.org/project/drupal_cms

🇺🇸United States fathershawn New York

All threads resolved - all tests passing :)

🇺🇸United States fathershawn New York

Confirmed! I can now remove a trait and a class - thank you @nod_ :)

I altered the render arrays in the test module to

    $build['replace'] = [
      '#type' => 'html_tag',
      '#tag' => 'button',
      '#attributes' => [
        'type' => 'button',
        'name' => 'replace',
        'data-hx-get' => $url->toString(),
        'data-hx-select' => 'div.ajax-content',
        'data-hx-target' => 'div[data-drupal-htmx-target="insert-here"]',
      ],
      '#value' => 'Click this',
      '#attached' => [
        'library' => [
          'core/drupal.htmx',
        ],
      ],
    ];

    $build['content'] = [
      '#type' => 'container',
      '#attributes' => [
        'data-drupal-htmx-target' => 'insert-here',
        'class' => ['htmx-test-container'],
      ],

Which produces data-hx-target="div[data-drupal-htmx-target="insert-here"]" through the standard string attribute. HTMX worked as expected.

🇺🇸United States fathershawn New York

That's a great question! I didn't think it would but I didn't try. I'll repeat the experiment with an attribute value selector and remove the complexity. Simpler is better!!

🇺🇸United States fathershawn New York

Applied a suggestion and will refactoring some improvements inspired by @larowan review. A maintainer should add him to the credits.

🇺🇸United States fathershawn New York

I had a conversation in Slack with @cilefen about how much we should present an abstract interface to the rest of the system and keep the HTMX implementation interface encapsulated. This alternate architecture would argue that we should name the facade class Ajax as that is the concept we are implementing. We would not provide direct access to the HtmxAttribute and HtmxHeader subsystems. The means of altering these subsystems would be to use an operation.

Such an architecture only exposes HTMX concepts to developers that need to create a combination of attributes and headers that is not available as an operation. In that case, the developer would create a new operation to encapsulate that combination. Of course such a developer could forsake the facade altogether and deal with the attribute and header subsystem directly.

This idea could be take further in 📌 Define and process an #htmx render array key Active and rather than define a new render array key, only add a new callback. The existing ajax callback would make an early return if the #ajax key did not point to an array and similarly the new callback if it did not point to an instance of the facade.

What is the right architecture?

🇺🇸United States fathershawn New York

Nice use of once property to verify the behavior had done its work +1!

With 10.2-alpha1 out should we move the tag to 10.3 and release all the functionality together?

🇺🇸United States fathershawn New York

New test passes.

🇺🇸United States fathershawn New York

Missed bringing over the Twig function

🇺🇸United States fathershawn New York

All tests green

🇺🇸United States fathershawn New York

All tests green

🇺🇸United States fathershawn New York

It might not be, but if we refactor BigPipe to use HTMX we should return HTML - probably with the placeholder as the main content, and I wondered if that might intersect with this use case??

🇺🇸United States fathershawn New York

I'm so glad to be working with collaborators who have a historical perspective on the code! This is an enhancement not a requirement that will give a bit of performance boost. I'm going to postpone it and move it down the task list before BigPipe.

In the module, this simplified HTML response is a route option and scanning the linked history that may be a good thing. Let's think about routes we might build to serve requests from HTMX and pick up this work in a while. Ideas that I have for routes are:

  • the one I have in the module /htmx/{entityType}/{entity}/{viewMode}
  • one to render placeholders as the main content
🇺🇸United States fathershawn New York

All other unresolved MR comments are from the previous MR which we have closed.

🇺🇸United States fathershawn New York

I can't mark the MR comment resolved but I removed the version requirement from the info.yml file in the test module.

🇺🇸United States fathershawn New York

@longwave Okay, that's two experienced maintainers recommending a renderer over page variants. Thanks for weighing in!

We don't need the query string as HTMX adds the header HX-Request: true to all requests. I'll start to work on that approach as an alternative. I think we do still want title though so that the HTMX feature of swapping the title is available if appropriate but we turn it off by default.

🇺🇸United States fathershawn New York

Rendering the system messages in the response would mean that we wouldn't need a command to define the messages. The core systems of defining and displaying messages is available.

We would need to implement in a later issue either

  1. Always select and display system messages as additional inserted content
  2. Provide an operation for selecting the messages from the response and displaying them to be used with appropriate
🇺🇸United States fathershawn New York

All tests green, including new unit test for this subscriber

🇺🇸United States fathershawn New York

Nice simple solution @nod_. All tests green including the the added Ajax interaction test. What else do we need to move to RTBC?

🇺🇸United States fathershawn New York

Added contacts to HTMX

🇺🇸United States fathershawn New York

Another advantage of the page variants is that SimplePageVariant contains system messages as well as main content. That would allow the calling element to use hx-select-oob, or for us to add an hx-swap-oob to the response, and display error messages as well as the selected content from the response.

🇺🇸United States fathershawn New York

Maybe. That doesn't necessarily look simpler to me but I don't know the internals as well as @larowlan. The work in 📌 Process attachments (CSS/JS) for HTMX responses and add drupal asset libraries Active is based on returning an HtmlResponse object, so would we be creating a simpler version of \Drupal\Core\Render\MainContent\HtmlRenderer? I would think we would still need an event subscriber to divert the request to the alternate renderer.

🇺🇸United States fathershawn New York

Opened an MR that uses a default simple page with a header control for using a block page variant.

🇺🇸United States fathershawn New York

fathershawn changed the visibility of the branch linting-in-progress to hidden.

🇺🇸United States fathershawn New York

fathershawn changed the visibility of the branch 11.x to hidden.

🇺🇸United States fathershawn New York

@nod_ requested two additional test cases in the prior PR.

  1. htmx powered elements inserted by our legacy ajax
  2. verify that our attach and detach behavior methods fire

I've added a route to the test_htmx module (/htmx-test-attachments/ajax) that creates markup for this test. In manual testing, htmx is inserted but does not process the inserted content. I think we need some glue code that calls htmx.process on markup inserted by our Ajax API.

The only way I see to verify that methods are called in the Nightwatch test is to use the callTracker assertion but this is deprecated, so I'm wondering if someone has a better approach?

I also added some documentation links to the JS files.

🇺🇸United States fathershawn New York

fathershawn changed the visibility of the branch without-htmx-attachement-processor to hidden.

🇺🇸United States fathershawn New York

fathershawn changed the visibility of the branch 3521173-process-attachments-cssjs to hidden.

🇺🇸United States fathershawn New York

fathershawn changed the visibility of the branch asset-tests to hidden.

🇺🇸United States fathershawn New York

Thinking about the feedback on an additional processor service I realized that we should be able to simplify that whole part of the solution by adding some appropriate conditions to the existing HtmlResponseAttachmentsProcessor. I'm hitting a wrinkle though which is that HtmlResponseAttachmentsProcessor gets called more than once preparing the response to my test page. This is an issue because if I call setAlreadyLoadedLibraries on the assets, then they stop getting added in subsequent runs.

🇺🇸United States fathershawn New York

Someone with more permissions must be needed to edit the JS Dependencies page. I don't know if someone edited source or there are other text filters available, but I can't add additional headers and rows to the existing table.

Here's the needed data:

Repository https://github.com/bigskysoftware/htmx
Release cycle Releases are expected quarterly.
Security policies https://github.com/bigskysoftware/htmx/security
Security issue reporting https://github.com/bigskysoftware/htmx/security/advisories/new
Contact(s) 1cg , fathershawn

🇺🇸United States fathershawn New York

I hav two questions about merging drupalSettings.

ajax.js uses jQuery to merge the incoming drupalSettings value using the deep variation of .extend()

      if (response.merge) {
        $.extend(true, drupalSettings, response.settings);
      } else {
        ajax.settings = response.settings;
      }

In my contrib implementation I didn't want to start with a reliance on jQuery so adapted code from https://youmightnotneedjquery.com. I've kept this pattern in the MR at the moment, but I see that jQuery.extend() is used nearly a dozen other scripts. What's the best way forward? I see #3179551: Provide no-library equivalents of common/useful jQuery functions but it doesn't include .extend().

2. The ajax API provides two options for drupalSettings above. I currently simply merge. What is the use case for not merging incoming settings?

I've added a Nightwatch test, and all Nightwatch tests are passing.

Production build 0.71.5 2024