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.

Sign up for GitHub

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

Jump to bottom

Fix code copying in Firefox #2120

Closed
wants to merge 5 commits into from
Closed

Fix code copying in Firefox #2120

wants to merge 5 commits into from

Conversation

wongjn
Copy link
Contributor

@wongjn wongjn commented Mar 1, 2025

Seems to be related to the text not having newline characters. Testing with v3 docs with <code> elements:

$0.textContent.replace('\n','\\n');

V3:

<!doctype html>\\n<html>\\n<head>\\n  <meta charset="UTF-8">\\n  <meta name="viewport" content="width=device-width, initial-scale=1.0">\\n  <link href="./output.css" rel="stylesheet">\\n</head>\\n<body>\\n  <h1 class="text-3xl font-bold underline">\\n    Hello world!\\n  </h1>\\n</body>\\n</html>\\n'

V4

<!doctype html><html><head>  <meta charset="UTF-8">  <meta name="viewport" content="width=device-width, initial-scale=1.0">  <link href="/src/styles.css" rel="stylesheet"></head><body>  <h1 class="text-3xl font-bold underline">    Hello world!  </h1></body></html>'

This PR:

  • Brings back the newline characters.
  • Post-processes the code snippets to remove extra newlines left over from the highlighter transforms.
  • Removes the display: block on the .line elements since we can now rely on the newline characters within the <pre> parent element's white-space: preserve behavior.

Tested in Firefox and Chrome on Windows for no visual regressions. Please test on Safari as I don't have access to that browser.

Fixes #1972.

wongjn added 3 commits March 1, 2025 15:40
@wongjn
Preserve line breaks
2724a4f
@wongjn
Remove extra empty newlines from code highlights
618e511
@wongjn
Remove double line spacing
c2ba9ff 
Newlines in `<pre>` break lines, so the `display: block` on each `.line`
would break the line again.
@vercel Vercel
Copy link

vercel bot commented Mar 1, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tailwindcss-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 3, 2025 5:17pm

philipp-spiess
philipp-spiess requested changes Mar 3, 2025
View reviewed changes
Copy link
Member

@philipp-spiess philipp-spiess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @wongjn! I did notice that there's some sort of regressions with multi-line highlights now though (left is current site right is your change):

Screenshot 2025-03-03 at 15 52 25

You can see that the width of the highlight no longer spans the whole code tag and that the lines no longer touch :/ I believe that was the issue we initially tried to fix with replaceAll("\n", "').

@wongjn
Copy link
Contributor Author

wongjn commented Mar 3, 2025

I'm sorry I didn't spot that 🙇

I've taken inspiration from how the Shiki docs do it, namely the width and display:

image

Before After
image image

@philipp-spiess
Copy link
Member

Thanks so much for looking into this! This change is awesome but I found another regression on Safari 😢 Looks like regular word highlights don't render at all anymore?

Screenshot 2025-03-04 at 16 33 24

@wongjn
Copy link
Contributor Author

wongjn commented Mar 4, 2025

Ah that's a bummer. I'm afraid I don't have a Safari machine to debug either, sorry :(

@wongjn wongjn mentioned this pull request Mar 4, 2025
Merged
@wongjn wongjn deleted the fix-firefox-code-copy branch March 5, 2025 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipp-spiess philipp-spiess philipp-spiess requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docs installation step wrong code format on copy
2 participants
@wongjn @philipp-spiess