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

CommentFileSizeAuthor
#14 3523727-nr-bot.txt3.76 KBneeds-review-queue-bot

Issue fork drupal-3523727

Command icon 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:

Comments

fathershawn created an issue. See original summary.

fathershawn’s picture

Status: Active » Needs review

All tests green

fathershawn’s picture

Issue summary: View changes
fathershawn’s picture

fathershawn’s picture

Title: DX object to manage htmx headers » Build and collect htmx headers
fathershawn’s picture

Issue summary: View changes
fathershawn’s picture

Status: Needs review » Postponed

We 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."

fathershawn’s picture

Status: Postponed » Needs review

Returning to this - we aren't using any headers in #3526267: Remove core/drupal.ajax dependency from big_pipe/big_pipe

fathershawn’s picture

Issue tags: +HTMX initiative
fathershawn’s picture

Issue summary: View changes
nicxvan’s picture

Took 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?

fathershawn’s picture

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

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new3.76 KB

The 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.

fathershawn’s picture

Status: Needs work » Needs review

All tests passing again

nicxvan’s picture

Ok 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.

nicxvan’s picture

Not sure if this requires a change record.

fathershawn’s picture

Questions answered and minor issues fixed :)

nicxvan’s picture

Status: Needs review » Reviewed & tested by the community

I 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.

fathershawn’s picture

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 #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.

fathershawn’s picture

Issue summary: View changes
fathershawn’s picture

fathershawn’s picture

Status: Reviewed & tested by the community » Needs work

Discussed 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

fathershawn’s picture

Issue summary: View changes
Status: Needs work » Closed (won't fix)

nod_’s picture

credits