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

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

Open
2 tasks done
gwynne opened this issue Aug 16, 2023 · 6 comments
Open
2 tasks done
Labels
bug Something isn't working

Comments

@gwynne
Copy link

gwynne commented Aug 16, 2023

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

  • If possible, I've reproduced the issue using the main branch of this package.
  • This issue hasn't been addressed in an existing GitHub issue.

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

c142759

@gwynne gwynne added the bug Something isn't working label Aug 16, 2023
@gwynne gwynne changed the title Embedding of custom icons via <use/> prevents working with complex SVG Embedding of custom icons via 'use' element prevents working with complex SVG Aug 16, 2023
@natikgadzhi
Copy link

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
Copy link
Author

gwynne commented Aug 21, 2023

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
Copy link
Contributor

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

@dobromir-hristov
Copy link
Contributor

dobromir-hristov commented Aug 22, 2023

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
Copy link
Contributor

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
Copy link
Author

gwynne commented Aug 22, 2023

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 😕.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants