Fix border-radius being applied on one side only for aligned images

[ltguillaume]

No description provided.

[yardenshoham]

Please provide some motivation and before/after screenshots

[ltguillaume]

Before

After

[yardenshoham]

Are these screenshots from gitea? I don't recognize the page

[ltguillaume]

It's just how part of a README.md is rendered in Gitea when using:

<img src="something.png" align="right">

(which is perfectly valid inside Markdown files)

[yardenshoham]

I see the issue, I wonder why it's the way that it is. It came from #15572

[ltguillaume]

Probably because GitHub doesn't use border-radius and it gives images a background color in order to not let <hr> elements go all the way up to the image (and even under transparent images).

Example:

Gitea

GitHub

We could also do that instead.

[wxiaoguang]

Played a while with these styles.

I can see that:

1. The Gitea border-radius style is provided by base.css : img { border-radius: 3px; }
2. The .markup img { padding-left / padding-right } will make the border-radius have no affect, or even wrong affect as the screenshot:
   • One side has border radius.
   • The other side doesn't have the border radius.

Since there is a global img { border-radius: 3px; }, I guess it's good to keep consistent with it, using .markup img { margin } to ensure the images have correct border radius.

So I think this PR does right (update: if it doesn't break anything else)

@silverwind how do you think?

[silverwind]

I'm not sure I understand the issue but changing padding to margin diverges from GitHub's CSS, which could cause more problems. Is there a demo repo to showcase the issue? Push it to both github.com and try.gitea.io please.

[wxiaoguang]

I think the author has demoed a bad case, see the first picture in the comment #23414 (comment)

* `<img src="something.png" align="right">`
* One side has border radius.
* The other side doesn't have the border radius.

[silverwind]

Why would margin or padding have a effect on border-radius? The former two affect the spacing outside the image element, the latter the image itself. I don't get it.

I think we should remove border-radius modification inside markup content. It's opinionated and I don't think other forges like GitHub do it, e.g.

.markup img {
  border-radius: initial;
}

We're already destroying certain image badges with that border-radius which aren't meant to have one.

[wxiaoguang]

I think we should remove border-radius modification inside markup content. It's opinionated and I don't think other forges like GitHub do it.

Maybe the global img { border-radius: 3px; } should be removed, instead of patching the styles again?

Does the global img { border-radius: 3px; } make sense?

[silverwind]

Does the global img { border-radius: 3px; } make sense?

IIRC, it's only so avatars look a bit nicer, so I would recommend reducing the rule to only match avatars.

[silverwind]

Probably because GitHub doesn't use border-radius and it gives images a background color in order to not let

---

elements go all the way up to the image (and even under transparent images).

I think this is also a good change that we can replicate. We should add opaque same-color background to markup images so they can break <hr> running through them.

[silverwind]

#23578 for the background change.

[silverwind]

@wxiaoguang wanna do the border-radius change? Then I'd say we close this one.

[wxiaoguang]

I think we could ask the author first.

@ltguillaume how do you think about the "image border radius" problem?

A summary of the above discussion: Instead of changing 'padding' to 'margin', a preferred approach would be removing (and fine-tune) the global img { border-radius: 3px; }.

If you have other opinions or solutions, please help to share your idea.

If you have interest about the approach above (removing the global img { border-radius: 3px; }), feel free to take it. Or maintainers/contributors could help to propose PRs.

[ltguillaume]

My preference would be removing the border-radius in Markdown renders and introducing background-color. So not the quick fix I provided with this PR, but a fix that resembles GitHub's behavior.

[wxiaoguang]

The background color has been fixed by #23578 (and following #23750)

For the border-radius, my preference (according to silverwind's suggestion, if I understand correctly) is to remove the global img { border-radius } and add .ui.avatar { border-radius }.

How do you think (and how to continue)?

[ltguillaume]

That makes perfect sense to me. Go for it!

[silverwind]

Yeah, let's do that change. It also matches GitHub behaviour, except they do set 50% border-radius on avatars to make them fully circle, but this is something I don't agree with as it cuts off too much from the image for no good reason.

So, let's just move the rule to avatars only and keep the existing small radius.

[wxiaoguang]

The "fixing border-radius" proposal is: Fix image border-radius #23886

[wxiaoguang]

@ltguillaume Thank you for your PR. Since #23886 / #23578 / #23750 have been merged, I guess the problem has been fixed?

[ltguillaume]

Yep, this can be closed!