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

Merge Requests

More

Recent comments

🇺🇸United States fathershawn New York

I pushed up a draft MR of how I imagine this implementation. I've commented out a condition that would depend on the upstream issue, but otherwise the single export form works here. My experience is that the form builder out of band swap needs to be deeper in the build process to prevent the validation error.

Nice edge case catch on the nested case with a full form swap @nod_!

🇺🇸United States fathershawn New York

What if we moved these lines and the associated ::processAssetLibraries method from HtmlResponseAttachmentsProcessor into an asset processor service?

     $ajax_page_state = $this->requestStack->getCurrentRequest()->get('ajax_page_state');
      $assets->setAlreadyLoadedLibraries(isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : []);
      $variables = $this->processAssetLibraries($assets, $attachment_placeholders);

We now have two situations where we would like to get just the asset markup: here and in BigPipe.

As a side note, we can also prevent direct access to this renderer by checking for the HX-Request header.

🇺🇸United States fathershawn New York

Discussed with @nod_ yesterday. He pointed out that we can communicate in a CR that themes need to apply the data-drupal-messages attribute to their wrapper element.

🇺🇸United States fathershawn New York

This is really good @nod_! Setting to "needs work" for a test for the new renderer. I'm also recommending that we hold FormBuilder changes for 📌 Support dynamic forms using HTMX Active

🇺🇸United States fathershawn New York

Thanks @nod_ !!

Removed the twig function from this issue's MR and the change record. It is extra.

CR is updated with the full picture of how all 4 issue changes work together.

🇺🇸United States fathershawn New York

@catch @larowlan @nod_ A CacheableMetada parameter was added to methods that take a URL based on feedback. This complicates calling these methods within Twig as the caching has already bubbled.

Does this change the utility of a providing the attribute object within Twig? Should we drop that function from this MR?

🇺🇸United States fathershawn New York

@nicxvan First, thank you for so carefully checking the tedious mundane details! Suggestions committed and test updated for the one method name fix.

Sanitization happens in the attribute value object's ::__toString method. They all extend AttributeValueBase which sanitizes the name property. We add AttributeJson in this MR, and follow this pattern.

🇺🇸United States fathershawn New York

Thank you @phenaproxima for improving the code with your questions and suggestions. I've resolved all comments with either updates or answers except those related to return types (see #30). All tests are passing.

🇺🇸United States fathershawn New York

@catch can you give guidance about adding return types to \Drupal\Core\Template\Attribute?

I hit a name collision on the BC policy with the Attribute name (As you and @nicxvan noted that was for PHP attributes). But it also seems to be permitted with these:

PHP and JavaScript classes
In general, only interfaces can be relied on as the public API. With the exception of base classes, developers should generally assume that implementations of classes will change, and that they should not inherit from classes in the system, or be prepared to update code for changes if they do. See also more specific notes below.

Public methods not specified in an interface
Public methods on a class that aren't on a corresponding interface are considered @internal.

We are adding an interface in this issue, but there no interface for this class now. @larowan seemed to think it was okay here: https://git.drupalcode.org/project/drupal/-/merge_requests/12097#note_51...

🇺🇸United States fathershawn New York

I brought the summary up to date with the code. We have 5 unresolved comment threads on the MR. Most open threads have a response.

Following a comment from @catch I updated the code in \Drupal\Core\Template\HtmxAttribute for:

  • ::get
  • ::post
  • ::put
  • ::patch
  • ::delete
  • ::pushUrl
  • ::replaceUrl

to pass in and instance of CacheableDependencyInterface and collect the cacheable metadata emitted by the Url object.

There is an open question about the advisability of adding return types in \Drupal\Core\Template\Attribute. I understand this class to be internal by policy but maybe that's not right?

The other 3 open threads all have answers.

I have time to move this project forward to let's get this to RTBC so it can be committed and work can start on the next layer!!

🇺🇸United States fathershawn New York

I very much affirm HTML as the data format.

In my last draft I was sending individual messages as HTML but I now think that all we need to do is render the messages normally using '#theme' => 'status_messages'. If the theme provides the selector for the wrapper element, with fallback to core div[data-drupal-messages] we can start get the innerHTML of the incoming wrapper and swap it into the existing wrapper in the DOM using the HTMX beforeend strategy.

🇺🇸United States fathershawn New York

Reading through the governance doc I am marking this for framework manager review since it is blocking other work.

🇺🇸United States fathershawn New York

We are also on a learning journey about the breadth of interactive hypermedia that HTMX can allow. I think we are going to enjoy how much interactivity we can create without the weight of an SPA framework.

🇺🇸United States fathershawn New York

That's true @nod_

I would return to my proposal in #23 that we bring the route option over from the contrib module.

🇺🇸United States fathershawn New York

Based on #12 and the description of the tag (A change record needs to be drafted before an issue is committed.) I'm removing the CR tag from this issue as these are sequential dependencies and the tag belongs on final component.

🇺🇸United States fathershawn New York

Based on #26 and the description of the tag (A change record needs to be drafted before an issue is committed.) I'm removing the CR tag from this issue as these are sequential dependencies and the tag belongs on final component.

🇺🇸United States fathershawn New York

Discussed and agreed in Slack that a single change record and release note will cover:

🇺🇸United States fathershawn New York

Discussed and agreed in Slack that a single change record and release note will cover:

🇺🇸United States fathershawn New York

Discussed and agreed in Slack that a single change record and release note will cover:

🇺🇸United States fathershawn New York

We are on a path to deprecate Ajax API. New functionality is now out of scope, however the new subsystem accepts any valid CSS selector by design. See 🌱 Gradually replace Drupal's AJAX system with HTMX Active .

🇺🇸United States fathershawn New York

We are on a path to deprecate this API. New features are now out of scope, but we will be implementing the spirit of this issue in the new subsystem. See 🌱 Gradually replace Drupal's AJAX system with HTMX Active .

🇺🇸United States fathershawn New York

Discussed and agreed in Slack that a single change record and release note will cover:

🇺🇸United States fathershawn New York

You called it @mxh! Fixed and backported to 1.4.x. Thank you both!

🇺🇸United States fathershawn New York

Tagged \Drupal\Core\Template\HtmlAttributeInterface as @internal as that seems consistent with the policy on Attribute class internals .

🇺🇸United States fathershawn New York

Changed my role

🇺🇸United States fathershawn New York

I think this is ready

🇺🇸United States fathershawn New York

Posting some notes and code snippets from conversations in Slack. I'm not using the issue fork as we aren't really ready for that yet as there are still dependencies that need to be committed.

I loaded up the 11.x branch locally and spent more time stepping through FormBuilder, FormAjaxResponseBuilder::buildResponse, and FormAjaxSubscriber::onException with xdebug.

  1. The form is definitely cached on an ajax interaction. This happens at FormBuilder.php:436
  2. I reversed the actual effect of FormAjaxSubscriber::onException. The Ajax classes copy the new build ID back to the form.

So I removed the previous code to maintain build_id from my POC branch and instead added the following immediately after the $form['form_build_id'] part of the form is built in FormBuilder::prepareForm.

    // If a form is building from an HTMX request and the form id has changed
    // add htmx attributes to update the build ID in the client.
    // @see \Drupal\Core\Form\EventSubscriber\FormAjaxSubscriber::onException
    // @see Drupal.AjaxCommands.update_build_id
    if ($this->isHtmxRequest() && $form['#build_id'] !== $input['form_build_id']) {
      $existing_id = Html::getId($input['form_build_id']);
      $htmx_attributes = new HtmxAttribute();
      $htmx_attributes->swapOob('outerHTML:input[data-drupal-selector="' . $existing_id . '"]');
      $form['form_build_id']['#attributes'] = AttributeHelper::mergeCollections($form['form_build_id']['#attributes'], $htmx_attributes);
    }

We don't have all the similar tools available yet, but this would be adding data-hx-swap-oob='outerHTML:input[data-drupal-selector="existing-form-build-id"]' to the build id input element on the response.

And that works! Validation is working, form_build_id is changing.

🇺🇸United States fathershawn New York

Noted in Slack that the object that is the main work of this issue is proposed to be composed into the object that is the main work of 📌 DX object to collect and manage HTMX behaviors Active which I think will be the most common context for interaction with the header object. I have the CR take on that issue so we can explain the whole picture.

🇺🇸United States fathershawn New York

Questions answered and minor issues fixed :)

🇺🇸United States fathershawn New York

Replace the table with an unordered list

🇺🇸United States fathershawn New York

Still trying to fix the anchor link symbol at The proposal

🇺🇸United States fathershawn New York

Fixing anchor link symbol location

🇺🇸United States fathershawn New York

This is a an un-reviewed page

🇺🇸United States fathershawn New York

testing menu links

🇺🇸United States fathershawn New York

Leaving documentation for others who might come across this issue. There is not an option to remove the message. There is an plugin property to allow display and a required method on plugins

  /**
   * Check the plugin definition for success_message or return a static value.
   *
   * @return bool
   *   Should a success message be displayed to the user?
   */
  public function displaySuccessMessage(): bool;

The provided base method defaults to false but plugins can provide their own logic or leave the default.

  /**
   * {@inheritdoc}
   */
  public function displaySuccessMessage(): bool {
    return $this->pluginDefinition['success_message'] ?? FALSE;
  }
🇺🇸United States fathershawn New York

Hello, working use hx-on:click, but use hx-on:intersect not working. Did I make a mistake?

I don't think so. As I said in #4 the intersect event might only work with trigger. If you really need to manipulate the DOM on intersection rather than request the needed markup, I think you will have to build the JS yourself. The core competency of HTMX is adding interactivity between your page and the server.

🇺🇸United States fathershawn New York

@nod_ suggested adding my name to the existing list of Ajax subsystem maintainers. I had pictured we were replacing one subsystem with another but it can also be described as replacing everything within the existing subsystem as HTMX is a ajax based library. Changed the issue name to match and opened an MR.

🇺🇸United States fathershawn New York

Add link to issues in review

🇺🇸United States fathershawn New York

I can give you some direction on how htmx can be part of the edge case that you are building, but first I wonder why? If the content is on the page and you want to reveal it then that is possible with just CSS. A quick search will yield you many resources.

HTMX may save you half the work if you want to trigger some manipulation of the current page. That is you don't have to write and attach the event handler to the element. You do still have to create the javascript that does the manipulation. All the htmx attributes that you use in your example are based around the core competancy of htmx, which is bringing in and using new content from a request.

You may be looking for hx-on* but I don't think the once modifier is available there.
However if you are replacing the element making the call, once may not matter.

<div hx-on:intersect="myMoveDivFunction('#related-links-backup')"></div>

You would have to create and attach the js with the myMoveDivFunction(). The intersect event is a custom HTMX event described in the docs for hx-trigger and is stated to be inherently a one time event. I don't know if it works with hx-on*

🇺🇸United States fathershawn New York

All tests passing again

🇺🇸United States fathershawn New York

Hi :)

There is no request in your second div. `hx-select` selects from the response returned from an htmx request.

See https://htmx.org/attributes/hx-select/

🇺🇸United States fathershawn New York

Improved \Drupal\KernelTests\Core\Http\HtmxHeaderTest::testJsonTriggers and added \Drupal\KernelTests\Core\Http\HtmxHeaderTest::testMerge

🇺🇸United States fathershawn New York

Steps to review

Copied from a Slack thread

  1. Check the post applied patches
  2. Review or run the command to confirm the process
  3. Spot check a few different ones to confirm the location and copy is correct

Checked the patches. All are cleanly applied unless noted.
&check; 0001-codesniff.patch
&check; 0002-manual-conversion.patch differs only in that visibility is added and a comment is edited in \Drupal\block\Hook\BlockHooks::themesInstalled
&check; 0003-testfix.patch
&check; 0004-deprecationfix.patch

I have my core dev setup running under DDEV so I copied in your command from the snippets page, created a new branch off 11.x and ran your command, responding Y to all options. The result differed only slightly from the MR branch in some comment formatting and the replacement of some procedural calls to t() with the \Drupal\Core\StringTranslation\StringTranslationTrait::t method.

Spot checked contents and attribute value of several hooks

🇺🇸United States fathershawn New York

It's dynamic page cache I think. The header cache is a good idea. I can see it used in one dynamic page cache entry but I'm still seeing a cached response.

🇺🇸United States fathershawn New York

Issue recreated in a test module

🇺🇸United States fathershawn New York

The expire time and token in AccessToken are set by the provider using the response from your oauth service.

If there's not expiration date nor refresh token then either they are not provided or you need a more custom provider

🇺🇸United States fathershawn New York

@jurgenhaas pointed out that we should call this "fixed" since it accomplished it's purpose!

🇺🇸United States fathershawn New York

Tagged new 2.0.2 release Thank you for the code contribution!!

🇺🇸United States fathershawn New York

We should have dropped Symfony 4 when D9 support was also dropped.

🇺🇸United States fathershawn New York

Thanks for this! FormAssemblyEntityViewBuilder uses Symfony\Component\DomCrawler\Crawler which uses Symfony\Component\CssSelector\CssSelectorConverter which is how we get to both dependencies.

🇺🇸United States fathershawn New York

This problem comes from the Ajax API which we are actively working to replace in 🌱 Gradually replace Drupal's AJAX system with HTMX Active . We decided to further decompose our refactoring of BigPipe, moving the PHP side to 📌 Refactor big_pipe module to remove dependencies on Ajax classes Active . I would still like feedback on the proposal #32 as we will need to render individual messages so that we can return HTML placeholders for messages when it is time to refactor.

🇺🇸United States fathershawn New York

Follow-up issue to "un-AJAXify the PHP side" created and linked in the sidebar

🇺🇸United States fathershawn New York

Thanks @nod_ for the clear explanation. :)

I'm happy to have learned about the internals of our BigPipe implementation. It was also worthwhile to understand how to remove the dependencies on the PHP side and that if we are to move to an HTML based approach, we need a way of rendering individual messages.

+1 for removing the ajax dependency on the client-side. I'm going to update the issue summary with some of what you wrote and change the issue title to reflect the scope.

Production build 0.71.5 2024