Skip to content

Favicon#350

Merged
mhils merged 3 commits intomitmproxy:mainfrom
denised:favicon
Feb 14, 2022
Merged

Favicon#350
mhils merged 3 commits intomitmproxy:mainfrom
denised:favicon

Conversation

@denised
Copy link
Contributor

@denised denised commented Feb 13, 2022

Added the favicon option as discussed.

The most interesting part was realizing that we need a media type to go with it. What I currently have implemented is a fairly fast and dirty function that will extract the type from the filename, for the most common file types only. It would fail on other file types, or on URI's that had query parameters (and probably other cases I haven't thought of). You may prefer something more robust, and/or exposing yet another option to allow the user to set the type themselves.

I also added a demonstration of overriding the {% favicon %} block to the custom-template example, which shows how to recreate the inlined svg favicon that was originally in use.

Finally, I made one of the snapshot examples use the favicon option (and updated the outputs of all snapshots).

@denised
Copy link
Contributor Author

denised commented Feb 13, 2022

Dunno what's going on with demo_eager --- I will check on it tomorrow.

denised and others added 3 commits February 13, 2022 12:26
Option accepts URI/path and auto-determines image-type for common formats from filename suffix.
Added demonstration of override of favicon to custom-template example.
Made one of the snapshots (demo_long) use the favicon option.
Updated outputs for all snapshots
@mhils
Copy link
Member

mhils commented Feb 13, 2022

Thanks for the PR! This was really useful, I would have missed a few things otherwise. 😃
According to https://html.spec.whatwg.org/multipage/semantics.html#attr-link-type, type is advisory, so that's a nice excuse to get rid of the image_type complexity. I've also wired up a --favicon option and marked the default favicon.svg for deprecation if custom templates still reference it.
Let me know if this looks good to you, I'd be happy to cut a pdoc 10 release once this is in. :)

@mhils
Copy link
Member

mhils commented Feb 13, 2022

Ah, also demo_eager was an artifact from #349. GitHub takes the CI config from the main branch, but runs it against your PR code, which still had the snapshots produced with an older Python patch release.

@denised
Copy link
Contributor Author

denised commented Feb 13, 2022

Looks good to me --- ship it! :-)

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.

2 participants