Clean up SVG

[techknowlogick]

The previous SVG was a combination of objects and paths. This PR cleans up the SVG.

[silverwind]

That inner border did look kind of nice on big logos, but I guess flat is the trend. Is the color change intentional?

Also, the new SVG does not include the detail-remove class anymore and I generally think we can drop that mechanism and you should apply this patch to do so:

Patch

diff --git a/Makefile b/Makefile
index 3c1304575..d5b707372 100644
--- a/Makefile
+++ b/Makefile
@@ -686,9 +686,9 @@ generate-gitignore:


 .PHONY: generate-images
 generate-images:
-	npm install --no-save --no-package-lock xmldom fabric imagemin-zopfli
+	npm install --no-save --no-package-lock fabric imagemin-zopfli
 	node build/generate-images.js

 .PHONY: pr\#%
 pr\#%: clean-all
diff --git a/build/generate-images.js b/build/generate-images.js
index 9b7b82017..c7f58f61d 100755
--- a/build/generate-images.js
+++ b/build/generate-images.js
@@ -2,9 +2,8 @@
 'use strict';

 const imageminZopfli = require('imagemin-zopfli');
 const {fabric} = require('fabric');
-const {DOMParser, XMLSerializer} = require('xmldom');
 const {readFile, writeFile} = require('fs').promises;
 const {resolve} = require('path');
 const Svgo = require('svgo');

@@ -39,25 +38,9 @@ async function generateSvgFavicon(svg, outputFile) {
   const {data} = await svgo.optimize(svg);
   await writeFile(outputFile, data);
 }

-async function generate(svg, outputFile, {size, bg, removeDetail} = {}) {
-  const parser = new DOMParser();
-  const serializer = new XMLSerializer();
-  const document = parser.parseFromString(svg);
-
-  if (removeDetail) {
-    for (const el of Array.from(document.getElementsByTagName('g') || [])) {
-      for (const attribute of Array.from(el.attributes || [])) {
-        if (attribute.name === 'class' && attribute.value === 'detail-remove') {
-          el.parentNode.removeChild(el);
-        }
-      }
-    }
-  }
-
-  svg = serializer.serializeToString(document);
-
+async function generate(svg, outputFile, {size, bg}) {
   const {objects, options} = await loadSvg(svg);
   const canvas = new fabric.Canvas();
   canvas.setDimensions({width: size, height: size});
   const ctx = canvas.getContext('2d');
@@ -92,9 +75,9 @@ async function main() {
   await generate(svg, resolve(__dirname, '../public/img/gitea-512.png'), {size: 512});
   await generate(svg, resolve(__dirname, '../public/img/gitea-192.png'), {size: 192});
   await generate(svg, resolve(__dirname, '../public/img/gitea-sm.png'), {size: 120});
   await generate(svg, resolve(__dirname, '../public/img/avatar_default.png'), {size: 200});
-  await generate(svg, resolve(__dirname, '../public/img/favicon.png'), {size: 180, removeDetail: true});
+  await generate(svg, resolve(__dirname, '../public/img/favicon.png'), {size: 180});
   await generate(svg, resolve(__dirname, '../public/img/apple-touch-icon.png'), {size: 180, bg: true});
 }

 main().then(exit).catch(exit);

Also, I guess something needs to be done about loading.png. Maybe convert it to a SVG that pulses on load or something else simple?

[6543]

the new color is brighter - should we realy change it?

[silverwind]

I'm fine with the color change personally. The border removal too as is not really noticable except on a fullscreen logo anyways.

[silverwind]

Can you move those fills to the path elements as fill attribute and then remove class on them?

[lunny]

@techknowlogick Could you paste some screenshots?

[silverwind]

@lunny click "Display Rich Diff" on the images in the diff.

[silverwind]

Looking at the diff once more, I can't help but feel the new logo is a downgrade. I think the new color looks a bit dull compared to the older more vibrant one and the removal of the border makes it seem lower quality as well to me.

Also, the "zoom in" will cause issues in systems like Discord that force a circle clipping of the image, I'd say we even need to zoom out the old logo a tiny bit further to prevent such clipping.

[techknowlogick]

For those who were unsure about the colour change, I've changed the colour back to the original. As for things such as discord clipping we can generate an image that that has appropriate image dimensions as different sites require different dimension requirements.

[codecov-io]

Codecov Report

Merging #13680 (81e7454) into master (c3fc190) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #13680   +/-   ##
=======================================
  Coverage   42.23%   42.23%           
=======================================
  Files         710      710           
  Lines       77233    77233           
=======================================
+ Hits        32617    32623    +6     
  Misses      39245    39245           
+ Partials     5371     5365    -6     

Impacted Files	Coverage Δ
modules/git/utils.go	73.77% <0.00%> (-3.28%)	⬇️
modules/process/manager.go	72.50% <0.00%> (-2.50%)	⬇️
modules/git/blame.go	65.71% <0.00%> (-1.43%)	⬇️
services/mailer/mail.go	60.21% <0.00%> (-1.08%)	⬇️
modules/git/repo.go	45.72% <0.00%> (-0.51%)	⬇️
models/issue_comment.go	52.71% <0.00%> (-0.16%)	⬇️
modules/notification/mail/mail.go	39.08% <0.00%> (ø)
models/error.go	39.31% <0.00%> (+0.32%)	⬆️
services/pull/check.go	51.82% <0.00%> (+0.72%)	⬆️
models/notification.go	67.24% <0.00%> (+0.98%)	⬆️
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3fc190...81e7454. Read the comment docs.

[lunny]

The new logo looks bigger than the old one and lack of border.

[silverwind]

Why not zoom out the SVG so it fits in a circle around the outer edges? I guess the difference would be hardly noticeable compared to current version.

[techknowlogick]

After having to edit a unique avatar sizing for discord, twitter, reddit, and github, I can tell you two things 1. I reaally dislike forcing circle avatars on users, and 2. by having the SVG as we have it now, allows us to scale the logo as needed, and not worry about extra spacing when calculating things.

[silverwind]

Hmm yeah I guess we want maximum available area, for things like favicon.

[kdumontnu]

@techknowlogick @silverwind is this PR still active/under review? It's blocking my PR. Anything I can do to help?

[silverwind]

No, seems good now. I didn't see it was ready.