Fix svg spacing

[kdumontnu]

This UI change adds spacing between the svg icon and text in header and reduce the left/right padding to prevent the tabs from wrapping.

Before:

After:

Before:

After:

[codecov-io]

Codecov Report

Merging #14638 (cbc8201) into master (487f2ee) will increase coverage by 0.09%.
The diff coverage is 98.36%.

@@            Coverage Diff             @@
##           master   #14638      +/-   ##
==========================================
+ Coverage   42.21%   42.31%   +0.09%     
==========================================
  Files         767      767              
  Lines       81624    81739     +115     
==========================================
+ Hits        34458    34586     +128     
+ Misses      41531    41520      -11     
+ Partials     5635     5633       -2     

Impacted Files	Coverage Δ
modules/structs/issue.go	0.00% <ø> (ø)
modules/queue/unique_queue_disk_channel.go	53.73% <50.00%> (-0.12%)	⬇️
modules/setting/setting.go	49.03% <100.00%> (ø)
modules/templates/base.go	42.30% <100.00%> (+1.13%)	⬆️
services/gitdiff/gitdiff.go	73.70% <100.00%> (+4.93%)	⬆️
models/login_source.go	27.70% <0.00%> (+0.26%)	⬆️
models/user.go	53.05% <0.00%> (+0.38%)	⬆️
models/gpg_key.go	54.07% <0.00%> (+0.56%)	⬆️
routers/api/v1/repo/pull.go	25.90% <0.00%> (+0.60%)	⬆️
... and 2 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 5e5b063...cbc8201. Read the comment docs.

[bagasme]

what did these mean?

[kdumontnu]

Those are simply the same as the padding top/bottom, copied from semantin.css

[bagasme]

@kdumontnu i don't see any visible changes on before/after comparison images here.

[zeripath]

Have to agree with @bagasme there - it's not obvious what the problem you're fixing is here.

[kdumontnu]

Have to agree with @bagasme there - it's not obvious what the problem you're fixing is here.

To be fair, it's a rather minor change. Added a short description above. The svgs should not be touching the text. This more closely matches how we style icons in other sections.

[6543]

@kdumontnu can you "Update Branch" please :)

[CL-Jeremy]

This has created some over-generalization issues. Take a look at the clone link panel on the repo page, for example, or the branch selector dropdown, which belong to the secondary menu class. Those child items are not direct children of the whole menu, but actually their own small menu class, and should obey their own rules.

So at least it should be > .svg to avoid recursion.

But again in your second example, on the top right corner, those children in the radio-style menu have also been affected by this. The spacing there was a lot smaller - though not necessarily optimal - and this change has made those gaps larger than expected.

Octicons were introduced in phases to replace FontAwesome, and are governed by different spacing strategies. Some are direct margins in stylesheets. Some are normal inline spacing with whitespace characters. Some are set with Flex boxes. The units are also a mix of em and px. And sometimes even overriding rules aren't made with such a general rule in mind, and might break as a result.

The icons of the top navbar (login, register, and dropdown items) are affected by this change, and has previously let me believe there was a design change in general (which I of course welcome, I very much appreciate your effort to fix these uniformity issues) and made the language switcher icon closer to the text in #14575. The dropdown menu itself now has one remaining FontAwesome icon that stands out as a result of this probably unintended override, which I thought was a miss, but turns out to be coincidental after all.

To be fair, these small things are not obvious to every pair of eyes and most people won't notice or bother to try and fix them, especially since the styling have gotten so many patches here and there, with loads of inconsistencies and !important hell. I just want to make my points clear, in hope that this kind of mistakes would happen fewer in the future.

I believe the correct fix would be to go through a checklist of spacing issues to be fixed, implement them locally in their respective scopes, and finally try to generalize bit by bit. Since there aren't "unit tests" for this kind of stuff, a lot of testing should be done in the browser's inspector, with measurements taken. Note that rules seen as sourced from semantic.css might actually be a bug due to code tidying, refactoring and dependency updates - which is frustrating and takes a long time to find out (for example, those overly wide labels everywhere: they were styled to be narrower since way back at Gogs and only broke until recently).

[techknowlogick]

@CL-Jeremy commenting on closed issues/PRs is a recipe for loosing comments to the ether, please open a new issue with these comments so that someone could follow up on them.