The badge in GitHub comment doesn't link correctly and doesn't apply dark mode

[frankie567]

When badging an issue, Polar bot can manually add a GitHub comment on our behalf to the issue.

While the badge in the initial message correctly follows dark mode, the badge added in the comment does not:

Looking at the source, they are exactly the same, except for one carriage return before the <!-- POLAR PLEDGE BADGE START --> comment (crazy 🤯).

This works with dark mode

You can pledge behind and help support this effort using Polar.sh
<!-- POLAR PLEDGE BADGE START -->
<a href="https://polar.sh/polarsource/polar/issues/868">
<picture>
  <source media="(prefers-color-scheme: dark)" srcset="https://polar.sh/api/github/polarsource/polar/issues/868/pledge.svg?darkmode=1">
  <img alt="Fund with Polar" src="https://polar.sh/api/github/polarsource/polar/issues/868/pledge.svg">
</picture>
</a>
<!-- POLAR PLEDGE BADGE END -->

You can pledge behind and help support this effort using Polar.sh

This does not work with dark mode

You can pledge behind and help support this effort using Polar.sh<!-- POLAR PLEDGE BADGE START -->
<a href="https://polar.sh/polarsource/polar/issues/868">
<picture>
  <source media="(prefers-color-scheme: dark)" srcset="https://polar.sh/api/github/polarsource/polar/issues/868/pledge.svg?darkmode=1">
  <img alt="Fund with Polar" src="https://polar.sh/api/github/polarsource/polar/issues/868/pledge.svg">
</picture>
</a>
<!-- POLAR PLEDGE BADGE END -->

You can pledge behind and help support this effort using Polar.sh

[frankie567]

Just noticed another thing: if you click on it, it opens the image but not the actual link.

GitHub definitely doesn't like the HTML comment on the line end 😅

[birkjernstrom]

Oh wow. Nice catch @frankie567! Will prioritize this today.

Found this gem of a discussion on the topic. Was interesting to find and learn about the [//]: # (This is a comment.) trick. At quick glance, I thought that might be the better and more markdown-esque correct approach.

However, I've been playing around with this and even with [//] newlines are crucial. Even more so as lack of newlines with [//] renders it as text vs. <-- --> where it's not shown, but can impact DOM nodes.

After some experimentation, I think GitHub simply treats these as an extension to what was written prior or next (depending on direction) if sufficient newlines are not there. In combination with still respecting classical HTML in all such nodes vs. [//] being considered as text in such cases. Causing our comment and a-tag to be "merged as one" during GitHub parsing and discarded (not recognized in their HTML mapping). As a result, there is no link and they default to creating a link to the SVG itself.

So be it [//] or <-- --> newlines are crucial. In addition, GitHub will truncate newlines between nodes to one so we can literally add "\n" * 10 before and after and visually it wouldn't make a difference in the end. 2 will suffice and should be sufficient to ensure this doesn't happen. Will keep <-- --> as a format since the current error is better than the comment being visible.

PS. Love that because you filed a bug about the badge and included the badge for illustrative purposes, Polar recognized the issue as badged and didn't badge it again 💪 However, tbh, I first thought it was a bug and got worried when I saw 2 badges 😅

[frankie567]

Interesting! Funny to see something that seem trivial at first glance is actually a complex subject 🙃

PS. Love that because you filed a bug about the badge and included the badge for illustrative purposes, Polar recognized the issue as badged and didn't badge it again 💪 However, tbh, I first thought it was a bug and got worried when I saw 2 badges 😅

Weeeell, to be 100% accurate, Polar did edit the issue. It didn't add the badge again but actually replaced the last (buggy) one. I had to re-edit to re-introduce the bug. You can see it in the edit history.

[birkjernstrom]

Hope to get to this today. Ran into an unexpected issue #873 and have another support ticket I need to get to 😅 Hopefully, I'll ship this later in the evening or tomorrow.

Funny to see something that seem trivial at first glance is actually a complex subject

I love those short lived little rabbit holes 😊

Weeeell, to be 100% accurate, Polar did edit the issue. It didn't add the badge again but actually replaced the last (buggy) one. I had to re-edit to re-introduce the bug. You can see it in the edit history.

Oh, right. Yes, I forgot it does check and updates in case it doesn't match the expected. But doesn't do it in case it's been embedded once (to avoid forcing it post manual edits).
https://github.com/polarsource/polar/blob/main/server/polar/integrations/github/badge.py#L196

[birkjernstrom]

Just a one-line fix 🤦🏼 Honestly, I thought it was going to be a bit more involved. I assumed it would require some tweaks to the core logic of how we badge which is always a bit sensitive to change.