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

Include panel scripts in content when RENDER_PANELS=True #1689

Merged

Conversation

matthiask
Copy link
Member

@matthiask matthiask commented Oct 22, 2022

It seems to me that panel scripts aren't included at all when using RENDER_PANELS=True at the moment. This would probably be a bug. I don't use RENDER_PANELS anywhere and therefore cannot really test if this change would be correct.

I found this while trying to write a test for the timer panel (because I wanted a green build, re. code coverage)

@matthiask
Copy link
Member Author

Addendum: My gut feeling is that this change is correct, I lean towards merging but I really want a second opinion.

@tim-schilling
Copy link
Contributor

RENDER_PANELS is used when setting up an asgi app with channels. We should extend the example app to support testing that case, but not necessarily now. I can run this PR against a project of mine that uses ASGI next week if you can wait till then.

@matthiask
Copy link
Member Author

matthiask commented Oct 22, 2022

Sure! No pressure at all. I added a template_source test here a8fa1d0 which already made the build green.

Without marking them as modules, they weren't being loaded in as expected.
@tim-schilling
Copy link
Contributor

Alright this should be good now. I spotted a bug where the "inserted" statement I removed (when it was still in the code) wasn't being printed out. Making it a module script seems to have resolved the issue. I'll leave the last review, squash and merge to you if you don't have any questions.

tests/test_integration.py Outdated Show resolved Hide resolved
@matthiask
Copy link
Member Author

No questions, thanks for checking and for improving the test!

@tim-schilling tim-schilling marked this pull request as ready for review October 23, 2022 13:29
@tim-schilling tim-schilling merged commit 4cf595c into jazzband:main Oct 23, 2022
@matthiask matthiask deleted the panel-scripts-and-render-panels branch October 23, 2022 14:30
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.

None yet

2 participants