This functionality is moved to #3524030: Provide a factory object for HTMX headers and attributes
Problem/Motivation
HTMX supports 11 custom response headers. Expecting Drupal developers to learn all the names and data requirements of all those headers would add to Drupal's learning curve.
Proposed resolution
Create a class with methods to abstract the details of these headers. These methods would both document and collect the necessary data.
This object could be used on its own but it is intended to be composed into an object attached to render arrays and processed into header attachments. That work will come in #3524030: Provide a factory object for HTMX headers and attributes.
Remaining tasks
A developer could use this object converted to an array and attach the headers to a response. The primary use of this object is to manage this data composed into an Htmx object that will be keyed to a render array property and processed in the rendering pipeline. This object will be created in a later issue.
User interface changes
None
Introduced terminology
None
API changes
None. HtmxHeaderInterface is added but headers are internal by policy.
Data model changes
None.
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3523727-nr-bot.txt | 3.76 KB | needs-review-queue-bot |
Issue fork drupal-3523727
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3523727-htmx-headers
changes, plain diff MR !12098
Comments
Comment #3
fathershawnAll tests green
Comment #4
fathershawnComment #5
fathershawnComment #6
fathershawnComment #7
fathershawnComment #8
fathershawnWe agreed in Slack to pause on this builder while we work on #3526267: Remove core/drupal.ajax dependency from big_pipe/big_pipe using headers that are "hand-rolled."
Comment #9
fathershawnReturning to this - we aren't using any headers in #3526267: Remove core/drupal.ajax dependency from big_pipe/big_pipe
Comment #10
fathershawnComment #11
fathershawnComment #12
nicxvan commentedTook a quick pass and I need to do some research, but had a couple of high level questions.
1. Is there a way to make the triggerJson method return a more dynamic message to ensure something didn't blow up so bad that nothing is happening? So per step?
2. I didn't see the merging explicitly tested, do we need to chain them? or did I just miss that?
Comment #13
fathershawnImproved
\Drupal\KernelTests\Core\Http\HtmxHeaderTest::testJsonTriggersand added\Drupal\KernelTests\Core\Http\HtmxHeaderTest::testMergeComment #14
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #15
fathershawnAll tests passing again
Comment #16
nicxvan commentedOk I went through this very carefully.
I read the linked docs and confirmed they all link correctly.
I confirmed the parameters.
I confirmed all headers are accounted for.
I had some questions that would help me understand this better.
I have a couple of questions on process for parameter order.
I think there is one test with markup that needs updating.
I confirmed the test module calls each and all of my previous questions about gaps have been resolved.
I think this is pretty much ready once the questions are answered and that one test module string is reviewed.
Comment #17
nicxvan commentedNot sure if this requires a change record.
Comment #18
fathershawnQuestions answered and minor issues fixed :)
Comment #19
nicxvan commentedI think this is ready.
There are a couple of discussions open worth confirming that or conclusions are accurate.
This looks like a logical step most of my review notes are in 16.
Open process questions I have are, does this need a CR and did this need a subsystem maintainer review?
I've posted this in slack for confirmation but marking based on my reviews.
Comment #20
fathershawnNoted 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 #3524030: Provide a factory object for HTMX headers and attributes which I think will be the most common context for interaction with the header object. I have the CR tag on that issue so we can explain the whole picture.
Comment #21
fathershawnComment #22
fathershawnDiscussed and agreed in Slack that a single change record and release note will cover:
Comment #23
fathershawnDiscussed in Slack with @nod_ (See comment on the attributes issue) and we are moving this work into #3524030: Provide a factory object for HTMX headers and attributes
Comment #24
fathershawnComment #26
nod_credits