Clean up SVG #13680
Clean up SVG #13680
Conversation
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 Patchdiff --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 |
the new color is brighter - should we realy change it? |
I'm fine with the color change personally. The border removal too as is not really noticable except on a fullscreen logo anyways. |
.st0{fill:#FFFFFF;} | ||
.st1{fill:#73A952;} | ||
</style> |
silverwind
Nov 24, 2020
•
Member
Can you move those fills to the path
elements as fill
attribute and then remove class
on them?
Can you move those fills to the path
elements as fill
attribute and then remove class
on them?
@techknowlogick Could you paste some screenshots? |
@lunny click "Display Rich Diff" on the images in the diff. |
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. |
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
The new logo looks bigger than the old one and lack of border. |
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. |
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. |
Hmm yeah I guess we want maximum available area, for things like favicon. |
@techknowlogick @silverwind is this PR still active/under review? It's blocking my PR. Anything I can do to help? |
No, seems good now. I didn't see it was ready. |
f8a668a
into
go-gitea:master
The xmldom dependency is no longer required since go-gitea#13680. Also, whitespace cleanup.
* Makefile cleanup The xmldom dependency is no longer required since #13680. Also, whitespace cleanup. * double the golangci-lint timeout
The previous SVG was a combination of objects and paths. This PR cleans up the SVG.