wimdeblauwe / htmx-spring-boot

Spring Boot and Thymeleaf helpers for working with htmx
Apache License 2.0
521 stars 49 forks source link

Handle flash attributes on htmx redirects #137

Closed xhaggi closed 3 weeks ago

xhaggi commented 4 weeks ago

This adds handling for flash attributes via RedirectAttributes. Now you can perform client-side redirects with htmx and the flash attributes work as with normal redirects.

Fixes #96

xhaggi commented 4 weeks ago

@wimdeblauwe there is a limitation, because the flash attribute processing does not work if HtmxResponse is set as model attribute (PR #128). I'm not sure if it was a good idea to support such a case and to be honest, I don't like it.

xhaggi commented 4 weeks ago

BTW @wimdeblauwe there is more magic done by RedirectView

We could support all of them to behave as similarly as possible to Spring. Let me know what you think about it.

checketts commented 4 weeks ago

@wimdeblauwe there is a limitation, because the flash attribute processing does not work if HtmxResponse is set as model attribute (PR #128). I'm not sure if it was a good idea to support such a case and to be honest, I don't like it.

I couldn't find an alternative approach that was compatible with using JTE Models as the return. I'm happy to try another approach if you have any suggestions.

xhaggi commented 4 weeks ago

@checketts let's move the discussion about an alternative approach to the initial issue #127.

wimdeblauwe commented 3 weeks ago

BTW @wimdeblauwe there is more magic done by RedirectView

  • Treat the redirect URL as a URI template (default: true)
  • Model attributes can be exposed as HTTP query parameters (default: true)
  • Propagate the query params of the current URL. (default: false)

We could support all of them to behave as similarly as possible to Spring. Let me know what you think about it.

If you are willing to implement that, it would be great if we align as much as possible with RedirectView I think.

xhaggi commented 3 weeks ago

If you are willing to implement that, it would be great if we align as much as possible with RedirectView I think.

Sure, but let's merge this before and I'll create a separate PR for it. Okay?

wimdeblauwe commented 3 weeks ago

ok, but can we already merge this? Don't we need a solution to #127 first?

xhaggi commented 3 weeks ago

I'm not sure because I need more information from @checketts on how he uses it in his projects. But if you ask me, we should revert #127 in favor of a better solution to his issue. It's definitely the wrong approach to render views in HtmxHandlerInterceptor#postHandle which was introduced by #127. And as far as I know, using JteModel as a return type is not a supported feature of Jte itself.

See https://jte.gg/spring-boot-starter-3/ or https://github.com/casid/jte/tree/main/jte-spring-boot-starter-3/src/main/java/gg/jte/springframework/boot/autoconfigure for more information about integration with Spring Boot.

wimdeblauwe commented 3 weeks ago

Ok, I agree that maybe it was bad judgement to have #127. If we merge this, it means @checketts is not blocked for now and we can have flash attributes, just not the combination of flashattributes and #127, right?

xhaggi commented 3 weeks ago

That's right.

checketts commented 3 weeks ago

I'm OK with that trade off. Sorry I've not been able to contribute much (been away working on building a physical building)

I'll re-open #127 and discuss the JTE specific needs and options there.