Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace text-decoration with bottom-border in links with ruby text #595

Merged
merged 2 commits into from
Feb 3, 2024
Merged

Replace text-decoration with bottom-border in links with ruby text #595

merged 2 commits into from
Feb 3, 2024

Conversation

stephenmk
Copy link

There's an annoying bug in WebKit that causes the text decoration line to split when the link contains ruby text that is wider than the base ruby character. See the link to "慎重" in the image below to see what I mean. The line underneath 重 is not wide enough.

chrome_before

(This bug doesn't affect Firefox.)

I propose replacing the text decoration line with a bottom-border when a hyperlink contains ruby text. You might ask why we should limit it to only the links that contain ruby text. If we use a bottom border style all the time, then the external hyperlink icons (visible in the lower right corner) will also be underlined.

I also suggest adding a small amount of padding (text-underline-offset) to all hyperlinks. This is the same amount of padding (0.2rem) that GitHub uses in its hyperlink styles. It can help prevent 十 from looking like 士, for example.

Here's an image of the same entry with my proposed new styles.

chrome_after

@stephenmk stephenmk requested a review from a team as a code owner January 30, 2024 04:03
Copy link

github-actions bot commented Jan 30, 2024

✔️ No visual differences introduced by this PR.

View Playwright Report (note: open the "playwright-report" artifact)

@stephenmk
Copy link
Author

By the way, I don't have my heart set on having the underline thickness set to 2px. I guess it's usually 1px, which might look better.

@stephenmk
Copy link
Author

OK, I updated the offset / thickness/ width values to derive from the --font-size-no-units variable as suggested. I also made the underline a little thinner than I had it before, which I think was a little too thick.

keisotu

@djahandarie djahandarie added this pull request to the merge queue Feb 3, 2024
Merged via the queue into themoeway:master with commit 16423a1 Feb 3, 2024
5 checks passed
@stephenmk stephenmk deleted the hyperlink-style branch February 3, 2024 05:01
@djahandarie djahandarie added the kind/bug The issue or PR is regarding a bug label Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug The issue or PR is regarding a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants