feat(convert/html): inline CSS and JS with convert --one_file --offline

[Rapsssito]

Description

Currently, the --offline parameter for convert stores the JS and CSS files in another folder. Inline CSS and JS makes it easy to share presentations by merging everything into a single HTML file. That way, combined with -cdata_uri=true, you don't need to share anything more than one HTML file.

EDIT: Changed -cdata_uri=true to --one_file. Now, the combination of --offline and --one_file together pushes the CSS and JS inline.

Check List

Check all the applicable boxes:

• I understand that my contributions needs to pass the checks;
• If I created new functions / methods, I documented them and add type hints;
• If I modified already existing code, I updated the documentation accordingly;
• The title of my pull request is a short description of the requested changes.

[jeertmans]

Hi @Rapsssito, it looks like a very interesting feature! Thanks!
However, I am not sure this should be the default, and might want to have an extra optional flag to have this, like « —onefile » or do, what do you think?

Also, could you create tests that ensure that the output html contains inlined content?

[Rapsssito]

@jeertmans, thanks for the feedback! I will work on the tests.

Regarding the implementation, I can set it up as a flag if you want (it is important to note that this new default behavior is not a breaking change). A parameter like --one-file, should also add -cdata_uri=true (which I haven't seen documented except in the FAQ) and this would cause redundancy and possible confusion. Maybe changing -cdata_uri to --one-file would be more beneficial and clearer?

[jeertmans]

Small comments on the test side

[jeertmans]

@jeertmans, thanks for the feedback! I will work on the tests.

Regarding the implementation, I can set it up as a flag if you want (it is important to note that this new default behavior is not a breaking change). A parameter like --one-file, should also add -cdata_uri=true (which I haven't seen documented except in the FAQ)

This is indeed not part of the documentation, but I plan to document config parameters better with #485, and probably also render them in the documentation.

and this would cause redundancy and possible confusion. Maybe changing -cdata_uri to --one-file would be more beneficial and clearer?

I was thinking of the following:

1. add --offline option, that downloads remote files;
2. add --one-file option, that acts as the previous -cdata_uri=true;
3. if --offline and --one-file as set, then JS / CSS files as inlined;
4. deprecate -cdata_uri flag, and remove it in Manim Slides v6.

Not sure if 3 is the best default behavior, but inlining everything in one HTML is not really a good habit, especially for videos as it creates very large HTML files.

What's your opinion?

[Rapsssito]

I have implemented your approach. Let me know what you think!

[jeertmans]

Hi @Rapsssito, here as some final reviews from me.

Could you also include documented those changes in CHANGELOG.md?

One in ADDED (about inlining CSS and JS) and one in CHANGED (about deprecating -cdata_uri in favor to -cone_file).

After that, I think it's ready to be merged, thanks!

[Rapsssito]

It should be ready to be merged!

[codecov]

Codecov Report

Attention: Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.56%. Comparing base (e50271b) to head (9bdbb22).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
manim_slides/convert.py	94.28%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #505      +/-   ##
==========================================
+ Coverage   79.27%   79.56%   +0.28%     
==========================================
  Files          23       23              
  Lines        1940     1962      +22     
==========================================
+ Hits         1538     1561      +23     
+ Misses        402      401       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jeertmans]

Thanks @Rapsssito! Some deadlinks were found, but this is because @PanoPepino recently changed its website URL, so I will fix that in another PR.

Once again, thanks for your great work!