-
-
Notifications
You must be signed in to change notification settings - Fork 7
Update web template and enable insert-based injection (Phase 4) #21
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
| <!-- Toolkit Style Insert --> | ||
|
|
||
| <!--@@ header:start @@--> | ||
| <!--@@ header:end @@--> |
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.
Strictly, this isn't just style - it could be any valid <head> content - css, JS, metadata tags (including OpenGraph content), icons, noscript fallbacks...
| /************************* Toolkit styles *************************/ | ||
| /*@@ CSS:start @@*/ | ||
| /*@@ CSS:end @@*/ | ||
|
|
||
| /******************************************************************* | ||
| * WARNING: Do not remove or modify this comment block, or add any | ||
| * content below this block. Briefcase will add content here during | ||
| * the build step. | ||
| * content below this block, including anything between the CSS | ||
| * markers. Briefcase will add content here during the build step. | ||
| ******************* Wheel contributed styles **********************/ |
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.
This comment is no longer accurate. If we're going to include a comment at all, it should:
- only refer to the CSS markers
- Occur before the CSS marker
- Be included as part of every CSS marker.
It doesn't need to be a big comment - a "WARNING: do not modify content between the @@css@@ markers" or similar would be fine.
| <!--@@ end:start @@--> | ||
| <!--@@ end:end @@--> |
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.
"end:end" is a bit ambiguous... "body-end" or "BodyEnd" (not sure what naming convention we're using here) might be better.
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.
Good point, thanks Russell
On that note, do you think it would it be worth having a set of "BodyStart" markers towards the top of the body?
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.
I can't think of an obvious reason you'd need one - but it doesn't cost anything to have the inclusion in case it's needed.
freakboy3742
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.
Looks good; one comment about another piece that can be pulled into Briefcase as part of making the tool "Pyscript agnostic".
| <script type="py" async="false" config="pyscript.toml"> | ||
| import runpy | ||
| result = runpy.run_module( | ||
| "{{ cookiecutter.module_name }}", run_name="__main__", alter_sys=True | ||
| ) | ||
| </script> |
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.
This is (obviously) a PyScript specific insertion - possibly not the actual Python code, but at the very least the <script> tag. It might be worth putting this into Briefcase as one of the insertions, on a body-python inclusion (and maybe rename the python inclusion as head-python)
freakboy3742
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.
👍 Looks great!
Holding off a merge until the final Briefcase piece is ready.
This PR implements Phase 4 of the referenced deliverable. It updates the Briefcase web template to use insert markers for HTML and CSS instead of hardcoded scripts and styles.
This decouples Briefcase from toolkit-specific dependencies and allows runtime assets to be owned and injected by GUI toolkits such as Toga.
Refs beeware/briefcase#2337
PR Checklist: