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

True SVG icon support #6027

Closed
5 of 6 tasks
rstoenescu opened this issue Jan 8, 2020 · 12 comments
Closed
5 of 6 tasks

True SVG icon support #6027

rstoenescu opened this issue Jan 8, 2020 · 12 comments
Labels

Comments

@rstoenescu
Copy link
Member

rstoenescu commented Jan 8, 2020

Synopsis

QIcons with SVG are already available but through images. This RFC is about true SVG icons, which will include svg Quasar icon sets and the ability to use true svg icons (so dynamically sizable and colorable) in devland too (through @quasar/extras package).

Advantages

  • Better app footprint -- only used icons will be included in the final build
  • Better quality icons
  • No need for including equivalent webfonts from @quasar/extras or CDN.

Devland usage

Notice in the example below that we want to avoid Vue observable wrapping, so we inject icons on the instance through created() hook. It will work if declared in data() too, but... overhead.

Also, the usage will extend to all icon-related props on Quasar components, which is why I've added a QBtn example too.

// some .vue file in devland
<template>
  <div>
    <q-icon :name="matMenu" />
    <q-btn :icon="mdiAbTesting" />
  </div>
</template>

<script>
import { matMenu } from '@quasar/extras/material-icons'
import { mdiAbTesting } from '@quasar/extras/mdi-v4'

export default {
  // ...
  created () {
    this.matMenu = matMenu
    this.mdiAbTesting = mdiAbTesting
  }
}

Custom SVG icons (not supplied by "@quasar/extras") will also be supported.

Quasar svg Icon Sets

Below is an example with Material Icons. It will also work with MDI, Fontawesome, ...

// quasar.conf.js
framework: {
  // ...
  iconSet: 'svg-material-icons'
}

QIcon svg name format

Svg icons will be defined as String with the following syntax:

Syntax: "<path>|<viewBox>" or "<path>" (with implicit 0 0 24 24 viewBox)
Examples:
  M9 3L5 6.99h3V14h2V6.99h3L9 3zm7 14.01V10h-2v7.01h-3L15 21l4-3.99h-3z|0 0 24 24
  M9 3L5 6.99h3V14h2V6.99h3L9 3zm7 14.01V10h-2v7.01h-3L15 21l4-3.99h-3z

QIcon with inline svg

QIcon can contain one <svg> tag (the content of the svg can be anything, not only a path). Reasoning on why to use an <svg> in a QIcon: the svg will respect the size and color as any QIcon through its props.

<q-icon color="accent" size="5rem">
  <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
    <path d="M0 0h24v24H0z" fill="none"/>
    <path d="M19 3h-4.18C14.4 1.84 13.3 1 12 1c-1.3 0-2.4.84-2.82 2H5c-1.1 0-2 .9-2 2v14c0 1.1.9 2 2 2h14c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2zm-7 0c.55 0 1 .45 1 1s-.45 1-1 1-1-.45-1-1 .45-1 1-1zm0 4c1.66 0 3 1.34 3 3s-1.34 3-3 3-3-1.34-3-3 1.34-3 3-3zm6 12H6v-1.4c0-2 4-3.1 6-3.1s6 1.1 6 3.1V19z"/>
  </svg>
</q-icon>

Some limitations:

  • do not use "height"/"width" attributes on the <svg> tag (it will brake QIcon's way of handling the size)
  • all <path>s will have "fill: currentColor" CSS applied by default; if you don't want that, then add fill="none" to the <path> tag

Current work status

Overall:

  • - QIcon support
  • - QIcon with inline svg
  • - Quasar SVG Icon Sets
  • - "@quasar/extras" svg build system
  • - "@quasar/app" support
  • - UMD variant usage

Per icon library:

Icon library Quasar Icon Set "@quasar/extras" svg build system
Material Icons 👍 👍
MDI v4 👍 👍
Fontawesome v5 👍 👍
Ionicons v5 👍 👍
Eva Icons 👍 👍
Themify Icons 👍 👍
@hawkeye64
Copy link
Member

    :aria-labelledby="iconName"
    role="presentation"

The resulting icon should have the above attributes set as well

@IlCallo
Copy link
Member

IlCallo commented Jan 9, 2020

Seems cool! Thank you for working on this feat.
First thing which comes to my mind is the input format: is it limited to a single path?
Many SVG I use in my projects are more complex than that and usually incapsulated into existing SVG files like /src/assets/logo.svg, because they are produced by a designer and imported in the project (separate files have the upside of allowing you to see the SVG using your IDE preview).

Example structure of the logo of a client of ours

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 84 60">
  <style>
.st0{fill:...}.st1{fill:...}.st2{fill:...}.st3{fill:...}.st4{fill:...}.st5{fill:...}.st6{fill:...}.st7{fill:...}.st8{fill:...}.st9{fill:...}.st10{fill:...}
  </style>
  <path d="..."/>
  <path d="..." class="st0"/>
  <path d="..." class="st1"/>
  <path d="..." class="st2"/>
  <path d="..." class="st1"/>
  <path d="..." class="st3"/>
  <path d="..." class="st4"/>
  <path d="..." class="st5"/>
  <path d="..." class="st6"/>
  <path d="..." class="st7"/>
  <path d="..." class="st8"/>
  <path d="..." class="st9"/>
  <path d="..." class="st2"/>
  <path d="..." class="st8"/>
  <path d="..." class="st10"/>
  <path fill="..." d="..."/>
  <path d="..." class="st1"/>
  <path fill="..." fill-rule="evenodd" d="..." clip-rule="evenodd"/>
  <g>
    <path d="..." class="st0"/>
    <path d="..." class="st1"/>
    <path d="..." class="st2"/>
    <path d="..." class="st1"/>
    <path d="..." class="st3"/>
    <path d="..." class="st4"/>
    <path d="..." class="st5"/>
    <path d="..." class="st6"/>
    <path d="..." class="st7"/>
    <path d="..." class="st8"/>
    <path d="..." class="st9"/>
    <path d="..." class="st2"/>
    <path d="..." class="st8"/>
    <path d="..." class="st10"/>
  </g>
</svg>

Of course this is probably a really complex use case (multiple fill colors, classes usage, groupings, etc), but it's also a good benchmark to check how powerful this feature could be.

Will be possible to reference SVG by file name? Or register them in advance (eg in a boot file) and use the name to reference them later on?

Angular Components MatIconRegistry API is pretty good and feature complete on that regard, I suggest to check its code.
addSvgIconSet was particularly powerful, it accepts the path of a file with this format

<svg>
    <defs>
        <svg id="icon-name-1" viewBox="0 0 24 24">
            <path d="..."/>
            <path fill-rule="evenodd" d="..." clip-rule="evenodd"/>
        </svg>

        <svg id="icon-name-2" viewBox="0 0 24 24">
            <path fill="none" d="..."/>
            <path fill-rule="evenodd" d="..." clip-rule="evenodd"/>
        </svg>

        <svg id="icon-name-3" viewBox="0 0 24 24">
            <path fill="none" d="..."/>
            <path d="..."/>
        </svg>
    </defs>
</svg>

and adds all icons into it.
Downside: you lose the ability of seeing all svg icons into your IDE preview because they are no more in separate files.

rstoenescu added a commit that referenced this issue Jan 9, 2020
@rstoenescu
Copy link
Member Author

Added "QIcon with inline svg" section.

@rstoenescu
Copy link
Member Author

@IlCallo I think you are referring to this.

@IlCallo
Copy link
Member

IlCallo commented Jan 10, 2020

Nope, <q-icon name="img:statics/image.svg" /> just translates to a <img src="statics/image.svg" />, it doesn't inline the SVG, so I have no control on the SVG fill color.

Keeping the SVG into a standalone file and being able to inline it helps reduce redundancy.

Little example from my current project: I have a custom Facebook SVG icon and I need it in the header menu, in the footer menu and in the drawer menu, so I keep it as an external file and inline it where needed using a webpack loader.

Possible minimal API for what I mean: <q-icon name="inline-svg:statics/image.svg" />, which translates to

<div class="q-icon">
  <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
    <path d="M0 0h24v24H0z" fill="none"/>
    <path d="M19 3h-4.18C14.4 1.84 13.3 1 12 1c-1.3 0-2.4.84-2.82 2H5c-1.1 0-2 .9-2 2v14c0 1.1.9 2 2 2h14c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2zm-7 0c.55 0 1 .45 1 1s-.45 1-1 1-1-.45-1-1 .45-1 1-1zm0 4c1.66 0 3 1.34 3 3s-1.34 3-3 3-3-1.34-3-3 1.34-3 3-3zm6 12H6v-1.4c0-2 4-3.1 6-3.1s6 1.1 6 3.1V19z"/>
  </svg>
</div>

If not done with a webpack loader (because it wouldn't allow reactivity) I guess it should be something like:

  • get the path
  • check it is an SVG
  • fetch the SVG with an HTTP call
  • clean previous SVG content (if any)
  • inject SVG content as an inline SVG

From what I could see of their codebase, this is also what Angular does under the hood of its MatIcon and MatRegistry.

@rstoenescu
Copy link
Member Author

rstoenescu commented Jan 10, 2020

There is a security concern at stake here. A BIG one. QIcon downloading something then rendering the content to DOM at runtime equals to zero security as the content can have anything. Which is why I told you about the "img:" scheme. @IlCallo

@IlCallo
Copy link
Member

IlCallo commented Jan 10, 2020

Yes, it opens up to XSS attacks if the dev uses it without caution.
Now I see why a registry is used in Angular: it keeps track of "safe" URLs in some way.
It's also written on the overview page:

In order to prevent XSS vulnerabilities, any SVG URLs and HTML strings passed to the MatIconRegistry must be marked as trusted by using Angular's DomSanitizer service.

I find this feature really valuable nonetheless (if all possible precautions are taken to avoid misuse), but I understand if you decide it's too time-consuming to implement it.
In my experience, I needed this feature for literally all projects I did (either 3-weeks websites, 1-year enterprise projects or something in the middle) with any framework (Angular, Vue or Quasar), so I guess it could be a common use case.

@tohagan
Copy link
Contributor

tohagan commented Jan 11, 2020

Dumb question ... So if you used an icon in a list, will it's contents get duplicated?
If so, then for large lists that would be a problem.

Plan B: I imaged that at build time, you'd detect and extract the icons in use and then create CSS for each of them. Same as existing icon fonts. That would remove the duplication and (potentially) be just a drop in replacement for existing icon references (no code changes). No external URLs required. Inlining would then only be in the CSS not in HTML. That also might loose/gain some features. This does not exclude the option of offering inline QIcon as well!

@rstoenescu
Copy link
Member Author

There is no duplication no matter how many times you use an svg icon and on how many vue components. Also, svg icons are not defined through CSS.

@rstoenescu
Copy link
Member Author

Available in “quasar” v1.7.0, so closing.

@eeerrrttty
Copy link

will this fix the q-icon loading as text instead of icons???

I want to fix for all

@rstoenescu
Copy link
Member Author

@eeerrrttty pls do not cross-post.. you will be answered on your initial ticket.

mesqueeb pushed a commit to mesqueeb/quasar that referenced this issue Feb 2, 2020
mesqueeb pushed a commit to mesqueeb/quasar that referenced this issue Feb 2, 2020
@rstoenescu rstoenescu unpinned this issue Feb 13, 2020
@IlCallo IlCallo mentioned this issue Nov 25, 2020
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants