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

Add mdbook-slide-evaluator #2258

Merged
merged 13 commits into from
Aug 21, 2024
Merged

Add mdbook-slide-evaluator #2258

merged 13 commits into from
Aug 21, 2024

Conversation

michael-kerscher
Copy link
Collaborator

@michael-kerscher michael-kerscher commented Aug 2, 2024

I created a first implementation for the mdbook-slide-evaluator I described in #2234.

This is still not finished but I wanted to provide a chance to review the current state.

One has to run a webdriver compatible browser (e.g. selenium-chromium) so the slides can be rendered. The browser can access the file either via a file:// or http:// uri.

The tool grabs the configurable element from that page and evaluates the size of this element. Output can be stored in a csv file or at stdout and looks like this at the moment:

$ mdbook-slide-evaluator book/html/android/aidl
book/html/android/aidl/birthday-service.html: 750x134
book/html/android/aidl/example-service/changing-definition.html: 750x555
book/html/android/aidl/example-service/changing-implementation.html: 750x786
book/html/android/aidl/example-service/client.html: 750x1096
book/html/android/aidl/example-service/deploy.html: 750x635
book/html/android/aidl/example-service/interface.html: 750x570
book/html/android/aidl/example-service/server.html: 750x837
book/html/android/aidl/example-service/service-bindings.html: 750x483
book/html/android/aidl/example-service/service.html: 750x711
book/html/android/aidl/types/arrays.html: 750x291
book/html/android/aidl/types/file-descriptor.html: 750x1114
book/html/android/aidl/types/objects.html: 750x1258
book/html/android/aidl/types/parcelables.html: 750x637
book/html/android/aidl/types/primitives.html: 750x366
book/html/android/aidl/types.html: 750x197

It is also capable of storing screenshots of the evaluated element to have a manual view.

Possible improvements:

  • use data:// uris in which the html page is encoded to avoid having complicated mount into a selenium docker container or needing to serve the pages via http. A problem is, that external files (e.g. css, etc.) are linked. With this data:// uri type the browser would not need any access to files or http.
  • have a look at included editors with rust code to see if these have a scrollbar (might be possible with some js code that returns this info)

Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty cool!

mdbook-slide-evaluator/src/slides.rs Outdated Show resolved Hide resolved
mdbook-slide-evaluator/src/main.rs Show resolved Hide resolved
mdbook-slide-evaluator/src/evaluator.rs Outdated Show resolved Hide resolved
@michael-kerscher
Copy link
Collaborator Author

I'm actually not sure if this repository is the best choice for this tool or if should be in a different/separate repo

@mgeisler
Copy link
Collaborator

Hey @michael-kerscher, let me just say that this looks super cool! It's impressive that you can get this done with so little code 😄 Thanks a lot!

I haven't had a change to try it out myself, but it seems the dependencies are just a Docker container with Selenium. That's cool and it should be something I could also run without Docker, right? Perhaps using python3-selenium from Debian? I'll eventually play around with this, but Docker is fine for GitHub CI.

@mgeisler
Copy link
Collaborator

I think you need to install dprint locally and run dprint fmt. We have instructions for that in our README or CONTRIBUTOR file.

@mgeisler
Copy link
Collaborator

I'm actually not sure if this repository is the best choice for this tool or if should be in a different/separate repo

It takes a few steps for us to create new repositories in the google/ GitHub organization, so I would suggest putting it into a subdirectory here for now. You're of course more than welcome to go through those steps and create a new repository for the tool! It just requires filling out some forms and so on...

@michael-kerscher
Copy link
Collaborator Author

Hey @michael-kerscher, let me just say that this looks super cool! It's impressive that you can get this done with so little code 😄 Thanks a lot!

I haven't had a change to try it out myself, but it seems the dependencies are just a Docker container with Selenium. That's cool and it should be something I could also run without Docker, right? Perhaps using python3-selenium from Debian? I'll eventually play around with this, but Docker is fine for GitHub CI.

You don't really need a docker container, yes.
You only need something that speaks webdriver and this can also be e.g. a native on your system managed by e.g. webdriver-manager that installs this on your system. python3-selenium is leveraging the locally installed e.g. chromedriver but does not bring it with itself. Both combine though.
I added an example to the Readme.md

@michael-kerscher
Copy link
Collaborator Author

What still needs some work is some actual policy that is evaluated. E.g. defining a threshold for how many pixels the height can be

@michael-kerscher
Copy link
Collaborator Author

To put some numbers here, the current full book with all pages that have a main element are evaluated in about 3 minutes when rendered with selenium/standalone-chromium:latest

real    3m3.411s
user    0m1.484s
sys     0m0.132s

@michael-kerscher michael-kerscher marked this pull request as ready for review August 16, 2024 11:07
@michael-kerscher
Copy link
Collaborator Author

This seems to be ready for the intended use case now.

@mgeisler
Copy link
Collaborator

This seems to be ready for the intended use case now.

Thanks for the example in the README, I'll give it a go!

Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this locally with the Python instructions and it's cool!

Let us merge this now so we have something to build on. Things to look at implementing later:

  • A small integration test for this, something we can run in GitHub to protect against bit rot. It could be something which evaluates a single static HTML file.
  • The GitHub integration which can be run on PRs to warn authors about too large pages. I think it would be useful if we could show the delta in size, something like "This PR increases the slide height by 20px". If we can do that, we can avoid annoying people when they fix typos in an already large slide.

mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
mdbook-slide-evaluator/README.md Outdated Show resolved Hide resolved
I think it will be useful to ignore a `.venv` directory, in case
people install the Python WebDriver tools into it.
@mgeisler mgeisler changed the title implemented mdbook-slide-evaluator Add mdbook-slide-evaluator Aug 21, 2024
@mgeisler mgeisler enabled auto-merge (squash) August 21, 2024 07:12
@mgeisler mgeisler merged commit 355d65b into google:main Aug 21, 2024
35 checks passed
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.

3 participants