- 🇨🇦Canada Charlie ChX Negyesi 🍁Canada
Note the FORMID constant was just a discussion in the issue, it's not found in the code yet. The attribution parameter is called
form_id.I marked the PR as non-draft.
- 🇺🇸United States nicxvan
It's what godotislate said in slack, I assume it means Out Of Scope.
- 🇺🇸United States nicxvan
Thanks guys. As far as the profile question it is resolved, I was on my phone so I couldn't see the changes super accurately.
collectModuleHookImplementations takes care of it I think so I've removed that.
I also want to document the .theme discussion from slack here a bit clearer.
From godotislate:
you can implement hook_form_alter, hook_page_attachments_alter, among a few others in .theme
but that is via theme manager, not module handler
it looks like they are all invoked after module handler has its turn
so keeping them OOS seems fine Yes. Also, only the author can select "Mark as ready", I think, so it would be a good idea to do so.
For Drupal core MRs, I don't know that marking them as "draft" really matters much, but often on other projects, reviewers are accustomed to skip reviewing draft MRs/PRs, because it's a signifier of WIP product not ready for review or to merge.
FYI, if you are the MR author, you can move it out of draft by clicking on the kebab menu in the upper right and selecting "Mark as ready".
@ghost-of-drupal-past If you click on the Merge Request (and make sure you are signed in to GitLab; just click Log In and it will automatically auth you), there should be an option to mark the MR as "Ready". That will remove the Draft status.
In addition, there should be a way to mark each of my comments as "resolved", either now or after you have committed a change that addresses a comment.
- 🇨🇦Canada Charlie ChX Negyesi 🍁Canada
Thanks for the meta review :)
- It's in draft because I have no idea what that means. I am just writing patches, here (in two weeks it'll be twenty years). This new workflow is a complete unknown for me.
- I talked with catch about a review , likely next week.
- I will
- Profile was addressed I thought?
- That's part of #1 I think
Edit: sorry I didn't see the review by solideogloria I will process that too.
- 🇫🇷France Grimreaper France 🇫🇷
Hi,
I tried to convert patch from comment 117 into a MR to more easily update it.
After trying to figure out in more details what it is was doing, I think I will open a separated issue for #3440128: version in info.yml for general project → .
In my case, I only want to try to extract versions from Composer if not present in .info.yml to provide site admin versions in admin pages.
- @grimreaper opened merge request.
- 🇺🇸United States nicxvan
I'm wondering if the merge request being in draft is intentional?
My understanding is that this is ready for a deeper review with the following comments that all need to be addressed.
1. A framework manager needs to review per 61.
2. The suggestion to change FormAlter(FormTestAlterForm::FORMID) to FormAlter(FormTestAlterForm::FORM_ID) per 63.
3. One question in the MR about handling the profile extension that I think may be addressed.
4. The question in the issue summary about whether additional tests are needed.Let me know if I missed anything, I added these to the issue summary too.
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇺🇸United States FatherShawn New York
@kevinquillen At the request of @nod_ we are discussing that in 📌 [POC] Implementing some components of the Ajax system using HTMX Active
- 🇺🇸United States kevinquillen
What is the easiest or smallest POC to proof out here?
- 🇺🇸United States FatherShawn New York
Some more thoughts on the questions raised in #43 🌱 [Plan] Gradually replace Drupal's AJAX system with HTMX Active , #44 🌱 [Plan] Gradually replace Drupal's AJAX system with HTMX Active , and #45 🌱 [Plan] Gradually replace Drupal's AJAX system with HTMX Active above:
However, Drupal pages have long been dependent on a strong relationship between
<head>and<body>content, with different JS/CSS bundles from page to page, for example.I think the existing concepts of
ajax_page_stateought to workI think I do have this working with
ajax_page_statein my contrib implemenation. HTMX includes a header,HX-Requestwith every request it makes. I have a response subscriber that only processes the response if this header was included on the request. On HTMX requests, the response is processed using logic adapted from our currentAjaxResponseAttachmentsProcessor. The resulting asset data is attached to the response as JSON.Now, you can indeed push changes to
<head>using multiple response top-level element with the hx-swap-oob attribute, but that needs an ID on each modified target, which needs to pre-exist on the source page, which seems like a lot of potential trouble.Perhaps HTMX has evolved it's capabilities as all my exposure is recent. The
<script>tag containing the JSON asset data is wrapped with<div hx-swap-oob="beforeend:body"></div>as part of the response processing. The value on hx-swap-oob defines the swap strategy and target that is used for the inner HTML of the<div>. The result is a script tag appended to the end of the body. HTMX fires an event,htmx:oobAfterSwap, after it has processed and inserted our asset JSON. Currently I have a listener for this event that uses theloadjslibrary we already use to process and attach the assets.which is likely to interfere with our use of behaviours
HTMX fires another event,
htmx:loadafter each response has finished processing. I'm currently using an event listener on this event to call ourDrupal.attachBehaviors(). All of this is verified in a test.I have less experience with the JS side of our core code and @nod_ said the JS here could be improved 🌱 [Plan] Gradually replace Drupal's AJAX system with HTMX Active so please offer improvements either in 📌 [POC] Implementing some components of the Ajax system using HTMX Active or in the HTMX module issue queue → .
A related issue will be history support. If we enable back/forward navigation with the hx-push-url we probably need to make it so that the cacheability headers, among others, be updated for updated pages, because the result will often not match the source. Think, for example, of a search page filled with results: the results will at least carry cache tags for the results but the original page will not.
If we did enable history support, what I think is important is that the url that is updated in the browser should be able to recreate the current display. Using the example of a search page, we commonly add query parameters to such urls containing the search terms. I don't believe that we are currently updating anything in the header on ajax responses, other than header JS included in the differential assets payload. If we find that we do want to add fully harmonizing the head section, we could explore adapting the HTMX head-support extension for our specific needs.
- 🇫🇷France fgm Paris, France
big pipe placeholders (which would be another thing to try a conversion of fairly early on
Indeed, that reminds me of another potential difficulty, which is the use of the
hx-trigger="load" or "revealed" or "intersect"relying on the default browsers events or the intersection API, which is likely to interfere with our use of behaviours to support multiple loads per context like document or other node e.g. with BigPipe. - 🇬🇧United Kingdom catch
However, Drupal pages have long been dependent on a strong relationship between and content, with different JS/CSS bundles from page to page, for example.
So this is true, but for AJAX requests we already deliver assets separately outside the
headmarkup and it's also the case for big pipe placeholders (which would be another thing to try a conversion of fairly early on since that relies entirely on the AJAX system with a small amount of custom js)..I think the existing concepts of ajax_page_state ought to work - i.e. we track the libraries that have already been sent, and when new libraries are sent, they go through the asset rendering system and are sent as a new aggregate - both script and stylesheet link tags are allowed in the body of the document so they can be sent with the HTML (or injected via js wherever). Drupal settings and HTML head links (favicon, feeds etc.) are a bit different though so definitely need to figure out those.
- 🇫🇷France fgm Paris, France
Before doing a lot with that, there's an issue with HTMX which I would not know how to solve for now : HTMX basically makes the assumption of a stable
<head>section across pages, except for the title: when elements are swapped by ahx-boost, possibly inheriting if from<body hx-boost="true">, all<head>content in responses is dropped except for the<title>element.However, Drupal pages have long been dependent on a strong relationship between
<head>and<body>content, with different JS/CSS bundles from page to page, for example.Now, you can indeed push changes to
<head>using multiple response top-level element with thehx-swap-oobattribute, but that needs an ID on each modified target, which needs to pre-exist on the source page, which seems like a lot of potential trouble.Even without using
<body hx-boost="true">, partial replacement responses are often likely to require<head>changes.I think this is something which needs to be solved earlier than later for a PoC, to outline the patterns that can be used to work around that discrepancy between the HTML model and the Drupal model.
- 🇦🇺Australia sime Canberra
My first encounter with htmx was watching theprimeagen review it through non-drupal lens. To me it felt like the DX is better than Drupal core and very much made me think about how comparitively hard Drupal ajax is.
- 🇫🇷France nod_ Lille
Thanks! That's an interesting project. Haven't had time to dig into the code but happy to see alternative takes being explored.
I'm personally for htmx because the philosophy is aligned with Drupal and what we try to do. And I like that a lot of things live in the markup instead of hidden away In a js object somewhere.
- 🇬🇧United Kingdom hugronaphor
Glad to hear there is a consensus that existing approaches to handle ajax are outdated and not DX friendly.
This is the exact reason why a while ago when I needed to build an SPA-like app, I needed something more mature and modern.I did look at that time at htmx but it felt to me that it's not the right solution to use especially that I have found out about Livewire. As a result I have ported it to Drupal as Wire.
The main benefit is that it's architecture has been tested and iterated for years already by Laravel community and it can handle any task mentioned here including form submissions, reacting/emitting events, web-socket, ... in fact Symfony ecosystem got their own Livewire inspired Live Components
To be clear, I'm not saying that I want my Wire in core instead of playing with htmx but I think we should look at the scope broader and consider something in between Wire and Symfony's Live components. Huge benefit on top of being able to create dynamic interfaces with ease, Laravel and Symfony developers would have a familiar tool in Drupal's core.
- 🇺🇸United States FatherShawn New York
Thanks for the support and clear guidance @nod_ :)
I opened 📌 [POC] Implementing some components of the Ajax system using HTMX Active
