Use icon sizes relative to font size

[pat-s]

fix #2162 (comment)

Icon are now relative to the font-size in the browser and the height is aligned with the text height.
New classes were added for the badge and forge/settings icons in the repo view, they didn't had any size limits applied before.

Tested with different font-size settings in FF, from 12 to 16.

Example with size 12 in FF:

Changes

• Removed unneeded extra padding around status bar icons and aligned them in height
• Aligned the pipeline info icons with the text size next to it
• Pipeline status and avatar icons have the same height now and are a substantially bigger than the text size of the commit message but a little bit smaller than before
• Forge icon and repo settings icon do now scale with the font size instead of being hard-coded in size
• Fix checkbox and radio button margins (and make them relative to font size)

[woodpecker-bot]

Deployment of preview was torn down

[xoxys]

Why are there such much diffs related to a changed property order? Makes it a bit hard to review.

[pat-s]

I think this comes from some vscode tailwind plugin I am using which automatically sorts the TW classes. I didn't configure this, it seems to be the default. I think it happens when auto-format is applied on file-save.

[xoxys]

Some parts look weird to me now. E.g. the status icon is now way too small.

After:

Before:

[xoxys]

Same on the pipeline list, this looks not good (status icon and avatar way too small):

Icon buttons are broken as well (changed aspect ratio that looks weird):

Before:

After:

[pat-s]

ok, let's discuss UI again 🙃️

To me they finally look "not broken" now 😄️ Here are my arguments:

Before, line-height of icons didn't match with the text and it always looked non-harmonic to me.
Now, icon-height = line-height, making the whole UI much more smooth to me.
Besides, I so far had the assumption that icon size was chosen somewhat randomly as we had different sizes all over the place (20px, 24px, 32px), also in places where IMO they should be identical.

Last, the forge and settings icons are way too big IMO and didn't had any size restrictions set at all. They now match in height with the line height of the owner/repo name to the left.

The same goes for the badge showing the pipeline status. Besides the fact that to me it isn't adding value at this place and could be removed entirely, it is way too large.

Overall my main point is though that all icons should have a relative size so the chosen font-size that "matches", i.e. either equals the font-size in line height or has a visible substantial difference (i.e. at least 1.5x).
Right now the difference is like 1.2x for font-size 16 (which is IMO way to small by default) and increases to 2-3x when reducing the font-size, as the icons don't scale alongside.

[xoxys]

I don't know what to say to be honest... Icon buttons should be squares and the focus on the pipeline should lay on the status icon... Just make all icons the same size related to any text around it doesn't make any sense at all to me and is a pure black and white perspective.

Right now the difference is like 1.2x for font-size 16

Sorry but this is some kind of "human bullshit" and I don't mean it in an offensive way, but odd numbers exists to use them. Just make everything even or dividable by 5 or something similar is some weird human obsessions.

[pat-s]

Your putting my comment somewhat out of context.

I didn't say "all icons should be identical" but some that match context-wise, being it with text attached right next to it (btw you even did #4458 because of that). But it doesn't make any sense from a UI PoV to have icons exceed neighboring elements by a fraction in height. There should be a logic behind sizing and no wild-west approach. Without such a logic, UI is "human bullshit" to me.

There are rules for building UIs (just to use the contrast ratio rules as an example). Similar ones apply to sizing and margins.

I am not bound to these sizes even though I consider them good but I'd like to see meaningful alternative proposals then that don't just follow a gut feeling.

[xoxys]

Ok if we want to strictly follow some ui design guidelines now fine but which one?

If you are looking at the before/after screenshots, do you really think it looks good? If we want to change the icon sizes, fine, but then the surrounding space is now way too much. The icons for the pipeline status in particular now just look lost.

I still think that especially icon buttons should have the same size regardless of the used icon. Now, every icon button as a completely different size.

[pat-s]

Ok if we want to strictly follow some ui design guidelines now fine but which one?

I am not a UI expert but if you check the icons and line height of this PR on GH, you'll also see that the icons on the right align with the line-height of the attached text.

Not everything needs to go into (this) one PR WRT to related changes (spacing, margins, etc).

I agree on the point of the surrounding space being to large somewhat. Yet again, I'd like to focus on the icon sizings here and if you don't like a 1:1 that's fine, but please come up with some other numbers than that are not 1.1x line-height like it has been before. Such relative sizes are confusing for the mind as they are so close to "match" but then they don't and your mind realizes this when looking at it and gives you the feeling that "something doesn't add up". If there is a clear difference (like e.g. 2x) then it is clear and non-confusing.

If you are looking at the before/after screenshots, do you really think it looks good?

Yes, much better than before WRT to sizings.

And I still think that especially icon buttons should have the same size regardless of the used icon.

I don't know yet what the "icon buttons" are in this context. Do you mean the forge icon? They should all have the same relative size now and the same height as the line-height of the owner/repo button on the left. If not, there's an issue.
In your screenshot I see that the Gitea icon looks good.

[xoxys]

The icon size before was just fine for me. Make it relative is fine but making them way smaller without adjusting the spacing looks ugly and unpolished to me and thats why I will not approve it. Of course if other maintainers agree on this PR I wont block it.

No the gitea icon doesnt look good. As I said these icons should be squares now the hight is bigger than the width and the width of the forge icon is different than the width of the settings icon button...

[pat-s]

No the gitea icon doesnt look good. As I said these icons should be squares now the hight is bigger than the width and the width of the forge icon is different than the width of the settings icon button...

Agree on that, height wasn't enforced by mistake.

[pat-s]

but making them way smaller without adjusting the spacing looks ugly and unpolished to me and thats why I will not approve it.

Fair enough. In this case I'll update everything and this PR will get bigger than initially thought, to have the possibility to potentially convince you with respect to styling.

[pat-s]

@xoxys I think the inner part of the radio button is too big with 0.6rem. What do you think about using 0.4rem?

0.6:

0.4:

[xoxys]

IMO 0.6 looks better and should be the same ratio we had before. But no strong opinion here.

[pat-s]

OK, then let's keep it. Are we good here finally? 🙂️

[xoxys]

Thanks for your work.

[qwerty287]

It looks like the inner circle of the radio buttons is not centered anymore?

Depending on the zoom level, it sometimes more on the left or the right / top or bottom.

[xoxys]

I guess thats a rendering issue in some browsers while using relative sizes like rem 😔

[pat-s]

@qwerty287 Which browser? Checked with FF, Brave and Safari and couldn't reproduce when zooming in/out.

[xoxys]

We can check if e.g. 0.5rem has the same issue or 0.65 might fix it.

[qwerty287]

Actually Firefox as well...

[pat-s]

My first check used a FF fork (Zen). I can reproduce with FF vanilla 👍️. However, given that it fails only for FF vanilla and works for others, I'd argue that this is a FF bug then? (and hence shouldn't block this PR).

[xoxys]

Have you tried the mentioned approaches?

[pat-s]

Not yet but now. Indeed 0.5rem seems to work. Did you have this issue before to know this potential workaround?
Still a strange bug IMO.

Setting to 0.5 then.

[xoxys]

Yeah, I had this sort of issue already. Good that this hack still works 🤣 Yes, really weird rendering bug.

Tested again, everything else looks good to me.

[pat-s]

Great we were able to finish this ✨