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

What do you think of from render_block import render_block_to_response? #59

Open
jillesme opened this issue Aug 17, 2024 · 8 comments · May be fixed by #60
Open

What do you think of from render_block import render_block_to_response? #59

jillesme opened this issue Aug 17, 2024 · 8 comments · May be fixed by #60

Comments

@jillesme
Copy link

Hi @clokep,

Lovely package you created! It's wonderful to use with HTMX.

What do you think of exporting render_block_to_response? I often do return HttpResponse(render_block_to_string(...)), would be nice to be able to just do render_block_to_response with the same args. I've created a helper utility in my own codebase, but I think other people might benefit from that too.

@robertpro
Copy link

This is what I was looking for, @clokep I can help submitting a PR if you accept this idea,

from render_block.shortcuts import render_block_to_response

def view(request):
    ...
    return render_block_to_response(request, template_name, block_name, context)

What do you think ?

@clokep
Copy link
Owner

clokep commented Aug 22, 2024

I think that would be OK, it should probably be named render_block to match render vs render_to_string?

(render_to_response is part of the view classes IIRC and not a standalone function.)

If you're going to PR, please include tests too.

@jillesme
Copy link
Author

jillesme commented Aug 23, 2024

@clokep / @robertpro I agree that it should probably be render_block to match render. Good call. I'll try to make a PR this week (of course, with tests). I underestimated my availability.

@gogognome
Copy link

Could you make the context available in the response so that in tests I can assert on the context instead of scraping the returned text?

@gogognome
Copy link

@jillesme do you think you can implement this soon? If not, then I am willing to create a PR for this issue.

@jillesme
Copy link
Author

Hi @gogognome please go for it. I do not have time right now.

@gogognome gogognome linked a pull request Sep 26, 2024 that will close this issue
@gogognome
Copy link

I created a pull request some time ago. Is there any chance that someone is going to review it?

@clokep
Copy link
Owner

clokep commented Dec 4, 2024

It's still on my to do list, I've unfortunately been fairly busy. Will get to it eventually. Sorry for the delay.

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 a pull request may close this issue.

4 participants