-
Notifications
You must be signed in to change notification settings - Fork 61
Handle htmx headers for @ResponseBody methods #183
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
Conversation
…ethods For methods annotated with @responsebody (or controllers with @RestController), the response is already commited before the interceptor is invoked, thus the headers won't be written. The ResponseBodyAdvice was added to handle these cases.
xhaggi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fchtngr looks good. Thank you 👍
xhaggi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase your branch onto main again.
| public RequestMappingHandlerMapping getRequestMappingHandlerMapping() { | ||
| return new HtmxRequestMappingHandlerMapping(); | ||
| public RequestMappingHandlerAdapter getRequestMappingHandlerAdapter() { | ||
| var adapter = new RequestMappingHandlerAdapter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add back the getRequestMappingHandlerMapping method? Otherwise, the request mapping is broken.
|
@fchtngr and could you please shorten the commit message |
|
@fchtngr When would you be able to do the changes? We would like to release a 5.0.0 with this included today or tomorrow. |
Handle htmx headers for @responsebody methods
|
@fchtngr I made the necessary adjustments and merged it in. Thank you for your contribution. |
|
@xhaggi @wimdeblauwe thx, i was away for a few days. thanks for updating and merging! |
@xhaggi the new PR based off of the main branch (instead 4.x)
see original PR here