swiftlang / swift-docc-render

Web renderer for Swift-DocC documentation.
Apache License 2.0
311 stars 51 forks source link

Embedding of custom icons via 'use' element prevents working with complex SVG #727

Open gwynne opened 1 year ago

gwynne commented 1 year ago

Description

When a custom icon for a given ID (such as technology) is set in theme-settings.json, the icon is embedded in the rendered page via SVG's <use/> element. This runs afoul of WebKit bugs which prevent non-trivial SVGs from rendering properly when embedded this way.

Checklist

Expected Behavior

I expect using a custom icon to allow for the full range of SVG features.

Actual behavior

As the custom icon is embedded via <use/> instead of being referenced directly or embedded verbatim, various WebKit bugs (see below) prevent using even "basic" SVG features like <linearGradient> or embedded <style>.

https://bugs.webkit.org/show_bug.cgi?id=258225 https://bugs.webkit.org/show_bug.cgi?id=65344 https://bugs.webkit.org/show_bug.cgi?id=104169 https://bugs.webkit.org/show_bug.cgi?id=179724 https://bugs.chromium.org/p/chromium/issues/detail?id=109212

Steps To Reproduce

  1. Visit https://api.vapor.codes/sqlitenio/documentation/sqlitenio/
  2. Compare to the actual SVG.
  3. Notice that the stroke gradient on the top "leaf" shape is only applied when accessing the SVG directly.

(Note: Numerous variations, such as using <defs>, <g>, etc. were tried; none worked.)

Swift-DocC-Render Version Information

c14275995772f6bad158af224cae668348c360ce

natikgadzhi commented 1 year ago

It looks like the custom icon gets rendered using use in https://github.com/apple/swift-docc-render/blob/95129994a3e5e3ccb48ad8cd2ca4fd38f91e9afe/src/components/SVGIcon.vue.

I have approximately zero experience with SVGs, but I wonder if we could inline the SVG probided by docc in <SVGIcon/> instead of using use. My gut tells me maybe it's tricky to inline the SVG file contents into a component because that would require some webpack config black magic?

gwynne commented 1 year ago

My thoughts exactly - maybe something like this? (Disclaimer: I have basically zero experience with Vue, this may be downright laughably wrong, my apologies if so.)

<template>
  <img
    aria-hidden="true"
    class="svg-icon"
    src="`${themeOverrideURL}`"
    width="100%" height="100%" 
    v-if="themeOverrideURL"
  />
  <svg
    aria-hidden="true"
    class="svg-icon"
    xmlns="http://www.w3.org/2000/svg"
    xmlns:xlink="http://www.w3.org/1999/xlink"
    v-else
  >
    <slot />
  </svg>
</template>

This approach would also allow using non-SVG icons and remove the requirement of ensuring the appropriate ID is set on the root element of SVGs.

ethan-kusters commented 1 year ago

CC: @dobromir-hristov @mportiz08 any thoughts here?

dobromir-hristov commented 1 year ago

So there are a few things we can and cant do with SVGs:

  1. In order to modify the color of SVGs, so they match the style of the website + light/dark modes, SVGs need to be inlined. We have a few ways to do this
    1. Inlining the entire SVG content, which means DocC would have to inject it into the app, which it cant really do.
    2. The <use> mechanism, but only if the icon is on the same domain as the app, and have the ID thing. That method comes with some browser limitations as you have found out, which we probably did not see in testing.
    3. The App would have to Async request each icon, read it and then inject it's content as inline HTML. This means delays, layout shifts etc.

If the path to the SVG is not in the same domain, there are CORS issues involved, so we are forced to use <img> tags. This is what we do now, inside the OverridableAsset.vue.

Going directly to img without the use, would mean users now have to maintain icons for both light and dark mode, as they would be essentially plain images.

@mportiz08 please pitch in, if you have any thoughts on this, or I made mistakes somewhere. I remember you worked on this feature mostly.

mportiz08 commented 1 year ago

Bummer. I wasn't aware of those existing bugs regarding <svg> and <use>, which is unfortunate because that is the approach we chose to try and best balance all those details that @dobromir-hristov outlined.

We considered using the <img> referencing the external SVG asset URL as you propose, but that has the major downside of us not being able to apply CSS for things like fill color, etc.

I don't have an alternate idea in mind at the moment, but it's probably worth considering if we should revisit how custom icons work if these limitations are as frustrating as you point out.

gwynne commented 1 year ago

I would be delighted to see the bugs in WebKit's handling of <use/> fixed instead; if it weren't for that, I'd have called it a delightfully elegant approach! But some of these issues have apparently existed for the entire lifetime of WebKit's SVG support despite numerous complaints, and even if they were solved, it would take who knows how long before the majority of users could benefit from it 😕.