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

x/website: spaces in preformatted blocks (as rendered from Markdown) are unintentionally collapsed #41507

Closed
bupjae opened this issue Sep 20, 2020 · 10 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bupjae
Copy link

bupjae commented Sep 20, 2020

What did you do?

Visited https://golang.org/ref/mod with Edge 85 on Windows 10

What did you expect to see?

Spaces in code examples should distinguished without problems.
(The following image is captured with modified stylesheet)

expect image

What did you see instead?

Spaces in code examples are too collapsed to read.

actual image

@cagedmantis cagedmantis changed the title doc: https://golang.org/ref/mod: spaces in code examples are too collapsed to read. x/website: golang.org/ref/mod: spaces in code examples are too collapsed to read. Sep 23, 2020
@gopherbot gopherbot added this to the Unreleased milestone Sep 23, 2020
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 23, 2020
@cagedmantis
Copy link
Contributor

/cc @dmitshur @andybons

@dmitshur
Copy link
Contributor

dmitshur commented Sep 23, 2020

This issue affects Chrome on macOS too:

image

There's some relevant CSS:

code {
  /* Reduce spacing between words in code inline with text. Monospace font
   * spaces tend to be wider than proportional font spaces. */
  word-spacing: -0.3em;
}

I think that was meant for inline code (like go get), but I don't think if it's intentional to apply that to preformatted text blocks.

/cc @jayconrod

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 23, 2020
@dmitshur dmitshur changed the title x/website: golang.org/ref/mod: spaces in code examples are too collapsed to read. x/website: spaces in preformatted blocks (as rendered from Markdown) are unintentionally collapsed Sep 23, 2020
@ipriver
Copy link
Contributor

ipriver commented Sep 24, 2020

According to MDN <code> tag is intended to indicate that the text is a short fragment of computer code. And @dmitris already mentioned as an example go get, so it should be incorrect to use this tag for such long code examples. Also, Go package docs don't use <code> tags to wrap their code examples and everything is fine there.

CSS option word-spacing: -0.3em; makes code look better e.g. if you wrap function test() with <code> it uses special font for that tag and there is a visible gap between "test" and "()".

The solution here should be to remove <code> tags from the code examples which are used inside <pre> tag at the https://golang.org/ref/mod page itself.

@dmitshur
Copy link
Contributor

dmitshur commented Sep 24, 2020

The solution here should be to remove <code> tags from the code examples which are used inside <pre> tag at the https://golang.org/ref/mod page itself.

As far as I know, the <code> tags in <pre> tags are generated by our Markdown renderer. (This needs to be confirmed.) If it is reasonably clean to modify the Markdown renderer (either by adjusting its configuration, or by updating to a newer version that improves this behavior), then I agree this is likely a better solution. Otherwise, there's no option but to fix this by changing the CSS selector.

@jayconrod
Copy link
Contributor

I can confirm those <code> tags are generated by the Markdown renderer. Or at least: they're not in the original Markdown source.

@benhoyt
Copy link
Contributor

benhoyt commented Oct 7, 2020

Yep, the <pre><code> tags for code blocks are generated by goldmark, see source. There's not a simple hook to tweak the tag output, but you can customize the "node renderer" and override those two methods ... however, it's pretty messy and requires reimplementing about 50 lines of Go code: diff of markdown.go to make this happen.

In contrast, the CSS fix is very simple, just requires adding a pre code { ... } override to reset the word-spacing back to 0 (tested in Firefox and Chromium on Linux):

diff --git a/content/static/style.css b/content/static/style.css
index 1adea2d..c4ffa39 100644
--- a/content/static/style.css
+++ b/content/static/style.css
@@ -98,6 +98,9 @@ code {
    * spaces tend to be wider than proportional font spaces. */
   word-spacing: -0.3em;
 }
+pre code {
+  word-spacing: 0;
+}
 pre {
   line-height: 1.4;
   overflow-x: auto;

So my suggestion would be to just run with what Goldmark produces (other Markdown renderers produce <pre><code> too) and just use the simple CSS fix.

@dmitshur
Copy link
Contributor

dmitshur commented Oct 8, 2020

Thank you for investigating this and sharing your findings @benhoyt. It's very helpful.

I agree with your suggestion. I would add one more optional part to it: it may be worth to also make a feature request in goldmark's issue tracker about changing its default HTML output, and see what they think.

I tried to look into CommonMark specification and it consistently shows the use of <pre><code> for code blocks, so maybe that's why many Markdown implementations output that. However, the Markdown specification is primarily about how to parse text as Markdown, not about how to render it to HTML/CSS, so there's a chance they'll agree it's simpler and cleaner to output only <pre> for code blocks, or explain why that isn't a good option.

@benhoyt
Copy link
Contributor

benhoyt commented Oct 8, 2020

Interesting re CommonMark spec. I've found several sites that suggest <code> inside <pre> is an indicator that what's in the pre-formatting block is actually code (as opposed to a poem, table, or whatever). Which is the case in most programming documentation, hence I guess why Markdown renderers typically do it this way. For example:

If you want to represent a block of source code in your HTML document, you should use a code element nested inside a pre element. This is semantic, and functionally, it gives you the opportunity to tell search engine crawlers, social media apps, RSS readers, and other interoperating web services that the content is computer code.

I guess that makes some sense. Though I'm sure the Markdown renderers just always do <pre><code> instead of trying to guess whether what's in the fenced block is actually code or not... and I suspect there'd be a fair bit of push-back to changing the Markdown rendering given the CommonMark spec and that it's so common.

In any case, seems like we have a simple CSS way forward.

@gopherbot
Copy link

Change https://golang.org/cl/271277 mentions this issue: content/static: fix collapsed spacing in preformatted blocks

@benhoyt
Copy link
Contributor

benhoyt commented Nov 18, 2020

@dmitshur Fix submitted at https://go-review.googlesource.com/c/website/+/271277

Before and after screenshots below. I've also tested that this doesn't mess up other documentation/spacing, for example the pkg documentation.

Current version:

Screenshot from 2020-11-19 09-27-49

After fix:

Screenshot from 2020-11-19 09-27-32

@golang golang locked and limited conversation to collaborators Nov 19, 2021
passionSeven added a commit to passionSeven/website that referenced this issue Oct 18, 2022
We have "word-spacing: -0.3em" for <code> in our CSS, but that causes
Markdown code blocks to have misaligned spacing. Markdown renders
blocks in <pre><code>, so this change updates the CSS to override the
word spacing back to zero for code blocks. (We use the goldmark
package, but other Markdown renderers produces <pre><code> too.)

Fixes golang/go#41507

Change-Id: Ic5262dfe006d1206843575975885deb1c14e5c75
Reviewed-on: https://go-review.googlesource.com/c/website/+/271277
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants