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

Tiny enhancement: make favicon an option #345

Closed
denised opened this issue Feb 9, 2022 · 5 comments
Closed

Tiny enhancement: make favicon an option #345

denised opened this issue Feb 9, 2022 · 5 comments

Comments

@denised
Copy link
Contributor

denised commented Feb 9, 2022

Problem Description

You can currently replace the favicon for a site by modifying the default frame template. IMO, users are unlikely to need to modify this template for other reasons, but likely to want to have a custom favicon.

Proposal

Make favicon an option ala logo.

I would be happy to do this work, if it seems worthwhile to do so.

@mhils
Copy link
Member

mhils commented Feb 9, 2022

I agree on making the favicon configurable via a CLI switch, not sure what's the best way to address it and what the default value should be1. A couple of options:

  1. A binary --favicon/--no-favicon switch. Folks can then turn off the default favicon and just place something in the default location (/favicon.ico) if they want to override it.
  2. A --favicon path switch. This would then insert <link rel="icon" href="$path"> into the HTML. The user would still need to place the file at the correct location.
  3. A --favicon filename switch. We would then inline the file, as we do currently.
  4. A --favicon filename switch. We would then copy the file to the documentation root and add <link rel="icon" href="$path">s.

Happy to hear what others are thinking!

Footnotes

  1. I don't think we need to show the pdoc logo by default.

@denised
Copy link
Contributor Author

denised commented Feb 9, 2022

I vote for (2) as the simplest to understand, and also matching the behavior of the existing logo option.
Leaving favicon as a block as it is now also gives users the option of implementing it a different way, if they choose.

Default value is a good question. You could just leave it blank, and let the browser show its 'no icon' value in that case.

@mhils
Copy link
Member

mhils commented Feb 9, 2022

I didn't consider the consistency with --logo, that's a very strong argument indeed. I think I am convinced. 👍

Leaving it blank by default sounds good to me as well. Please feel free to send a PR! 😃

@denised
Copy link
Contributor Author

denised commented Feb 9, 2022

I will be happy to do that!

@mhils
Copy link
Member

mhils commented Feb 14, 2022

This is now fixed in pdoc 10, thanks again! 🍰

@mhils mhils closed this as completed Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants