Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make params argument of trigger_client_event() optional #263

Merged

Conversation

cjtapper
Copy link
Contributor

@cjtapper cjtapper commented Sep 1, 2022

This PR adds support for triggering a client event with no params without having to call the trigger_client_event with a params argument. It also adds support for using string rather than JSON value for the HX-Trigger* when there is only a single event with no params (see https://htmx.org/headers/hx-trigger/).

# Before this PR
>>> response = HttpResponse()
>>> response = trigger_client_event(response, "showConfetti", {})
>>> response.headers["HX-Trigger"]
'{"showConfetti": {}}'

# After this PR
>>> response = HttpResponse()
>>> response = trigger_client_event(response, "showConfetti")
>>> response.headers["HX-Trigger"]
"showConfetti"

# Also handles promoting of single events to JSON if additional events are added
>>> response = HttpResponse()
>>> response = trigger_client_event(response, "showConfetti")
>>> response.headers["HX-Trigger"]
"showConfetti"
>>> response = trigger_client_event(response, "showMessage", {"value": "Hello!"})
>>> response.headers["HX-Trigger"]
'{"showConfetti": {}, "showMessage": {"value": "Hello!"}}'

I chose to set the default value of promoted events to {} rather than None (or something else), as this mirrors how HTMX itself handles events with no params (it sets the details to an empty object; see https://github.com/bigskysoftware/htmx/blob/60acaacc416812e85245f8e00d1e8bc4954d1d83/src/htmx.js#L992-L1008)

I realise that the docs will also need to be updated - I'm happy to do this as well, but just wanted to check that this would actually be accepted first :)

@lolrenx
Copy link

lolrenx commented Sep 1, 2022

Good stuff, the empty trailing payload has been triggering me for a while

Copy link
Owner

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's quite complicated to try supporting the non-JSON form of the header. It looks like we could just send HX-Trigger: {"event": null} or {"event": {}} if params aren't provided, which is a much smaller change?

@cjtapper
Copy link
Contributor Author

cjtapper commented Sep 1, 2022

Would certainly simplify things. Supporting the non-JSON form is probably an unnecessary micro-optimisation, though one advantage of it is that it's slightly less cumbersome to assert for in tests.

We'll want to use {"event": {}} rather than {"event": null} if we want HTMX to behave exactly the same way as it would for the non-JSON version.

@adamchainz
Copy link
Owner

Would certainly simplify things. Supporting the non-JSON form is probably an unnecessary micro-optimisation, though one advantage of it is that it's slightly less cumbersome to assert for in tests.

Tests will just assert for the raw string {"event: {}}, no?

We'll want to use {"event": {}} rather than {"event": null} if we want HTMX to behave exactly the same way as it would for the non-JSON version.

Okay, right. Can you do it then? Please also rebase, and add changelog note and docs 😇

@cjtapper
Copy link
Contributor Author

cjtapper commented Sep 1, 2022

Yep, will have some time to make these changes a bit later today

@cjtapper cjtapper force-pushed the trigger-client-event-no-params branch from 692e8f8 to d051a8e Compare September 1, 2022 08:33
@cjtapper cjtapper force-pushed the trigger-client-event-no-params branch from d051a8e to fe64ddd Compare September 1, 2022 08:44
@cjtapper cjtapper changed the title Add support for single client events with no payload Make params argument of trigger_client_event() optional Sep 1, 2022
@cjtapper
Copy link
Contributor Author

cjtapper commented Sep 1, 2022

Well, this has ended up being a much smaller diff. I guess it saves me from typing four extra characters every time I call trigger_client_event with no params 😅.

There wasn't an example in the changelog for how you like to list consecutive PRs from the one contributor, so I just included the same "thanks to..." message a second time.

@adamchainz
Copy link
Owner

Thank you very much!

There wasn't an example in the changelog for how you like to list consecutive PRs from the one contributor, so I just included the same "thanks to..." message a second time.

Yup this is good.

@adamchainz adamchainz merged commit 03a15b3 into adamchainz:main Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants