Skip to content
This repository has been archived by the owner on Oct 1, 2021. It is now read-only.

fix: removes the id in all svg icons #664

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

alfonsobries
Copy link
Contributor

Summary

Removes the id on all svg icons to prevent issue described on ticket

Related: ArkEcosystem/explorer#941
Ticket: https://app.clickup.com/t/1jktfgf

Checklist

  • I checked my UI changes against the design and there are no notable differences
  • I checked my UI changes for any responsiveness issues
  • I checked my (code) changes for obvious issues, debug statements and commented code
  • I provided a screenshot of my changes to the component (if applicable)
  • I regenerated the icons.html file and checked if my newly added icon is shown correctly (if necessary)
  • I added an explanation on how to use the component to the readme (if necessary)
  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

faustbrian and others added 3 commits September 24, 2021 19:54
Optimized SVG(s):
- resources/assets/icons/brands/abra.svg
- resources/assets/icons/brands/android.svg
- resources/assets/icons/brands/ark_black.svg
- resources/assets/icons/brands/atomic.svg
- resources/assets/icons/brands/blog.svg
- resources/assets/icons/brands/crypto_com.svg
- resources/assets/icons/brands/deployer.svg
- resources/assets/icons/brands/exodus.svg
- resources/assets/icons/brands/linux.svg
- resources/assets/icons/brands/macos.svg
- resources/assets/icons/brands/nodem.svg
- resources/assets/icons/brands/outline/bitbucket.svg
- resources/assets/icons/brands/outline/discord.svg
- resources/assets/icons/brands/outline/gitlab.svg
- resources/assets/icons/brands/outline/telegram.svg
- resources/assets/icons/brands/windows.svg
- resources/assets/icons/categories.svg
- resources/assets/icons/cog-2.svg
- resources/assets/icons/copy.svg
- resources/assets/icons/cross.svg
- resources/assets/icons/document-success-2.svg
- resources/assets/icons/dollar.svg
- resources/assets/icons/double-checkmark-box.svg
- resources/assets/icons/download.svg
- resources/assets/icons/edit.svg
- resources/assets/icons/errored.svg
- resources/assets/icons/exclamation-mark.svg
- resources/assets/icons/filter.svg
- resources/assets/icons/info.svg
- resources/assets/icons/key.svg
Copy link
Contributor

@leMaur leMaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alfonsobries I wrote a bunch of comments but those can be extended on all svgs...

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 44"><g id="_4" transform="translate(-1347 -738)"><path id="Фигура_2_копия" d="M1347 775.1c0-16.4 6.7-22.1 12-22.1s12 5.6 12 22c0 7.2-4.9 7.9-7 6-1-1-2.3-5-5-5s-4 4.1-5 5.1c-.7.6-1.5.9-2.4.9-2.2 0-4.6-1.9-4.6-6.9zm6-31.1c0-3.3 2.7-6 6-6s6 2.7 6 6-2.7 6-6 6-6-2.7-6-6z" fill="currentColor"/></g></svg>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's boring to do but I think IDs like those may clash too...

Suggested change
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 44"><g id="_4" transform="translate(-1347 -738)"><path id="Фигура_2_копия" d="M1347 775.1c0-16.4 6.7-22.1 12-22.1s12 5.6 12 22c0 7.2-4.9 7.9-7 6-1-1-2.3-5-5-5s-4 4.1-5 5.1c-.7.6-1.5.9-2.4.9-2.2 0-4.6-1.9-4.6-6.9zm6-31.1c0-3.3 2.7-6 6-6s6 2.7 6 6-2.7 6-6 6-6-2.7-6-6z" fill="currentColor"/></g></svg>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 44"><g transform="translate(-1347 -738)"><path d="M1347 775.1c0-16.4 6.7-22.1 12-22.1s12 5.6 12 22c0 7.2-4.9 7.9-7 6-1-1-2.3-5-5-5s-4 4.1-5 5.1c-.7.6-1.5.9-2.4.9-2.2 0-4.6-1.9-4.6-6.9zm6-31.1c0-3.3 2.7-6 6-6s6 2.7 6 6-2.7 6-6 6-6-2.7-6-6z" fill="currentColor"/></g></svg>

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 1200 1201"><defs><path id="SVGID_1_" d="M0 0h1200v1201H0z"/></defs><clipPath id="SVGID_2_"><use xlink:href="#SVGID_1_" overflow="visible"/></clipPath><g id="Logo_ARK.io_Black"><g id="logo" transform="translate(-655 -116)"><path id="Rectangle" d="M715 116h1080c33.1 0 60 26.9 60 60v1080c0 33.1-26.9 60-60 60H715c-33.1 0-60-26.9-60-60V176c0-33.1 26.9-60 60-60z" fill="currentColor"/><path id="ARK" d="M1253.4 608.2l-456.2 472.3 456.2-730 456.1 730.1-456.1-472.4zm-135.3 245.9H1388l77.9 80.6h-425.1l77.3-80.6zm56.8-58.3l78.5-81.3 79.4 81.4-157.9-.1z" fill="#212225"/></g></g></svg>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fun of xlink:href="" it can cause strange error on page loading that can be difficult to guess, if the ID clash with another in the page.
In this case I'd prefer treat it manually by copy/pasting it on https://jakearchibald.github.io/svgomg/ and check the result. 90% of the time you end up with an identical icon with less markup. But again it's so boring...

For example, this is the optimized version from svgomg ⬇️

Suggested change
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 1200 1201"><defs><path id="SVGID_1_" d="M0 0h1200v1201H0z"/></defs><clipPath id="SVGID_2_"><use xlink:href="#SVGID_1_" overflow="visible"/></clipPath><g id="Logo_ARK.io_Black"><g id="logo" transform="translate(-655 -116)"><path id="Rectangle" d="M715 116h1080c33.1 0 60 26.9 60 60v1080c0 33.1-26.9 60-60 60H715c-33.1 0-60-26.9-60-60V176c0-33.1 26.9-60 60-60z" fill="currentColor"/><path id="ARK" d="M1253.4 608.2l-456.2 472.3 456.2-730 456.1 730.1-456.1-472.4zm-135.3 245.9H1388l77.9 80.6h-425.1l77.3-80.6zm56.8-58.3l78.5-81.3 79.4 81.4-157.9-.1z" fill="#212225"/></g></g></svg>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1200 1201"><defs><path id="a" d="M0 0h1200v1201H0z"/></defs><path d="M60 0h1080c33.1 0 60 26.9 60 60v1080c0 33.1-26.9 60-60 60H60c-33.1 0-60-26.9-60-60V60C0 26.9 26.9 0 60 0z" fill="currentColor"/><path d="M598.4 492.2 142.2 964.5l456.2-730 456.1 730.1-456.1-472.4zM463.1 738.1H733l77.9 80.6H385.8l77.3-80.6zm56.8-58.3 78.5-81.3 79.4 81.4-157.9-.1z" fill="#212225"/></svg>

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 46.2 44"><style>.st0{fill:currentColor}</style><g id="_1" transform="translate(-1402 -740.004)"><path id="Фигура_1" class="st0" d="M1432.8 744.2c-3.3-6.2-13.9-5.1-16.5 0S1402 784 1402 784l8.8-2.2s10-27.6 11-30.9 4.1-4.8 5.5-1.1 12.1 30.9 12.1 30.9l8.8 3.3s-13.7-36.6-15.4-39.8z"/><circle id="Эллипс_1" class="st0" cx="1425.1" cy="769.6" r="5.5"/></g></svg>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually prefer inline styles and rid off the <style> block completely. Less noise...
Here again I higly suggest to remove weak IDs like id="_1", id="Фигура_1" (Ellipse_1) and id="Эллипс_1" (Figure_1)

Suggested change
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 46.2 44"><style>.st0{fill:currentColor}</style><g id="_1" transform="translate(-1402 -740.004)"><path id="Фигура_1" class="st0" d="M1432.8 744.2c-3.3-6.2-13.9-5.1-16.5 0S1402 784 1402 784l8.8-2.2s10-27.6 11-30.9 4.1-4.8 5.5-1.1 12.1 30.9 12.1 30.9l8.8 3.3s-13.7-36.6-15.4-39.8z"/><circle id="Эллипс_1" class="st0" cx="1425.1" cy="769.6" r="5.5"/></g></svg>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 46.2 44" fill="currentColor"><g transform="translate(-1402 -740.004)"><path d="M1432.8 744.2c-3.3-6.2-13.9-5.1-16.5 0S1402 784 1402 784l8.8-2.2s10-27.6 11-30.9 4.1-4.8 5.5-1.1 12.1 30.9 12.1 30.9l8.8 3.3s-13.7-36.6-15.4-39.8z"/><circle cx="1425.1" cy="769.6" r="5.5"/></g></svg>

@leMaur leMaur self-assigned this Sep 27, 2021
@leMaur leMaur marked this pull request as draft September 27, 2021 07:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants