Keeping our markup consistent

[PeterShaggyNoble]

Following on from my side note in this comment, I've run a few different RegExes with various degrees of strictness in order to identify inconsistencies in the markup of all our existing icons. The first thing to decide is which of the issues we want to fix in those and then which ones we want to lint for in the future. Ideally, where possible, as most of these are merely minor niggles, we would try to fix these automatically. This list does not include the fix from #3239.

1. Space before the > in the opening svg tag
   The 3 files affected are: ardour.svg, centos.svg & cloudsmith.svg
2. Attributes on opening SVG tag out of order
   There are 365 files affected by this To be addressed in Automated fixes in Pull Requests #2380
3. Space before or after the = in one or more of the opening svg tag's attributes
   jetbrains.svg is the only file affected by this, with a space after role.
4. No closing Z or z in the path's data
   There are 88 files affected by this one
5. Space before the closing / in the path tag
   The four files affected by this are: celery.svg, deepin.svg ktm.svg & ublockorigin.svg.

[PeterShaggyNoble]

Forgot to hit "Comment" after writing up my own feedback on this last night! 🙄

1, 3, & 5 are ones I think we should definitely fix (I have a branch ready to go with those changes) and add to the linting, perhaps by expanding on the test added in #3239.

2 is not something I think we need to enforce, despite my own, strong preference for attributes to be consistently sorted in alphabetical order. If we were to require it, it's definitely something that should be automated (ref: #2380), rather than adding another hoop for contributors to jump through, as everyone has their own preferences when it comes to sorting attributes.

On 4, I have it in the back of my head somewhere that there is some software out there that chokes on SVGs that contain a path that isn't closed with a final Z but, given that we've received no complaints so far, I could well be wrong. Still, though, it could fall into the "better to have it and not need it than need it and not have it" category. Again, it's something that I'd suggest should be automated.

For the linting, my suggestion would be to modify the RegEx on line 6 to something along the lines of:

const svgRegexp = ^<svg( [^\s]*=".*"){3}><title>.*<\/title><path d=".*"\/><\/svg>\r?\n?$

and then rewording the error message on line 80

[ericcornelissen]

Regarding 1, 3, and 5 I personally don't feel like it is necessary to enforce this without us being able to automatically fix it. In contrast to #3239, these does not really affect the end user. The primary gain, I would say, is a slight reduction in file size of the SVGs. The consistency here, I think, is less important than easy contribution, unless the error message can be made very clear.

Regarding 2, SVGO can do this for you and it will with our configuration. At least a consistent order, I'm not sure if it is alphabetical.

Regarding 4, I'm not too familiar with this but if 1) the standard says you should or 2) there is software that requires it (even if the standard doesn't) I think we should enforce including it. As you said, its "better to have it and not need it than need it and not have it".

[PeterShaggyNoble]

I personally don't feel like it is necessary to enforce this without us being able to automatically fix it.

Oh, absolutely, the more we can automate, the better for all contributors. And, come to think of it, this is something SVGO would take care of, too. I'll get a PR in for the fixes, anyway.

1. the standard says you should or 2) there is software that requires it (even if the standard doesn't) I think we should enforce including it. As you said, its "better to have it and not need it than need it and not have it".

I'll do some research on this this evening but, the fact that SVGO doesn't add it if it's missing leads me to believe that I may have been wrong in my recollection.

[NovaGL]

Hi Peter since this topic already exists, what's you opinon on the xml you added in a previous submission.
Should it be allowed if people want to keep the xml tags in there? If it's ok maybe the submission text should be changed?

[PeterShaggyNoble]

@NovaGL, note that the SVG file you're referring to above is for use as a favicon on our website and isn't part of our library. Our library should cater to the "lowest common denominator". For example, people inlining the SVGs in their HTML don't need the additional markup. Anyone who does need i can easily write their own tooling to add it.