search

r-lib / pkgdown

Generate static html documentation for an R package
https://pkgdown.r-lib.org/
Other
718 stars 336 forks source link

Logo is no longer displayed if SVG version is present #1746

Closed klmr closed 3 years ago

klmr commented 3 years ago

find_logo preferentially finds an SVG logo, if present.

However, copy_assets always copies that logo file to a file called package-logo.png. This causes the browser to not render the logo at all, and no logo is displayed (Safari additionally displays a “missing image” placeholder).

The PNG version should probably be preferred, if present: it’s going to have been rendered from the SVG and is more portable (for instance, Inkscape by default includes text spans in SVGs without embedding the font, which will therefore render differently if the client hasn’t got the font installed). I’d submit a PR but I don’t actually know how to handle the SVG case: package-logo.png is currently hard-coded in multiple places. Should it be replaced by a function call? Or should pkgdown attempt to render the SVG into a PNG itself?

I think the break was introduced in #1584. That PR also makes the displayed logo smaller on all pages (including the homepage): 100px, instead of previously 120px (or whatever was specified), at least when using Bootstrap v4. Was this intentional?

maelle commented 3 years ago

That does look like a bug indeed, thanks for catching this and identifying the source of the problem.

I've started working on a fix in #1753. Your questions are good. :sweat_smile:

hadley commented 3 years ago

I think if you provide an SVG you're responsible for providing one that renders well everywhere (i.e. picking between a png and svg logo is outside of pkgdown's responsibilities). The proposed fixed in #1753 looks good to me.