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

fix: do not wrap code blocks #6083

Closed
wants to merge 3 commits into from
Closed

Conversation

lex111
Copy link
Member

@lex111 lex111 commented Apr 27, 2022

Summary

Problem

Code blocks are using soft-wrap for long lines, which makes the code harder to read and understand. In general, this is not a recommended practice for docs, since code blocks should be scrollable since it also makes the page longer and prevents the user from seeing the code as usual way.

Solution

Revert to default behavior for pre element.


Screenshots

Before

image

After

image


How did you test this change?

  1. Open the page with blocks of code with long lines, eg https://developer.mozilla.org/en-US/docs/Web/CSS/:where

@caugner
Copy link
Contributor

caugner commented Apr 27, 2022

Hi @lex111, we require that all commits are signed. Please see the documentation about signed commits and how to sign yours on GitHub.

In the mean-time, I will mark your PR as draft. Please mark it as ready as soon as your commit is signed.

@caugner caugner marked this pull request as draft April 27, 2022 16:35
@lex111
Copy link
Member Author

lex111 commented Apr 27, 2022

@lex111 I see, done it ✔️ Please check it out.

@caugner caugner marked this pull request as ready for review April 27, 2022 17:33
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

Thank you for your PR!

Unfortunately this degrades the usability on larger screens (see screenshots below).

Please maintain the current behavior for pointer devices (@media (hover: hover)), distinguishing them from touch devices (@media (hover: none)). See: https://developer.mozilla.org/en-US/docs/Web/CSS/@media/hover

Before

image

After

image

@lex111
Copy link
Member Author

lex111 commented Apr 27, 2022

I don't really understand, what's wrong with it? This is expected behavior, isn't it? Usually docs sites don't use non-disabling soft-wrap for code blocks for all screens.

@caugner caugner added the 🧑‍🤝‍🧑 community contributions by our wonderful community label May 12, 2022
@schalkneethling
Copy link
Contributor

Please maintain the current behavior for pointer devices (@media (hover: hover)), distinguishing them from touch devices (@media (hover: none)). See: https://developer.mozilla.org/en-US/docs/Web/CSS/@media/hover

👋 Hey Claas, are you suggesting we keep soft wrapping for desktop and just disable it for mobile and tablet devices? I do not think we should soft-wrap code inside code examples. It would be great if we were able to run these through Prettier so that long lines are generally avoided. For now though, I reckon just not wrapping is OK.

I believe on content folks generally try to avoid really long lines in general. @Rumyra, thoughts?

@caugner
Copy link
Contributor

caugner commented May 19, 2022

@schalkneethling If we disabled soft-wrap on desktop, the :where example would become less usable, because it is too long to fit on a screen and the scrollbar only appears at the bottom (the only way to scroll horizontally would be to select the text, but that's a very uncommon pattern):

Screen.Recording.2022-05-19.at.18.16.17.mov

@Rumyra
Copy link
Contributor

Rumyra commented May 20, 2022

I think this is another case for a linter https://developer.mozilla.org/en-US/docs/MDN/Guidelines/Code_guidelines/General#code_line_length - not sure if there is a historical discussion around these guidelines

@OnkarRuikar
Copy link
Contributor

Scrollbars are not constantly visible on safari. If whitespace/indentation falls on right border then readers may not even realize that there is more code to the right.

Linting sounds good. But it'll still wrap on low resolution screens.

Can we show return arrow (↵) where it wraps?

@caugner
Copy link
Contributor

caugner commented May 21, 2022

Automatic lining of the code blocks' line length would indeed solve this, so once this maximal line length (64 characters?) is enforced, we can safely disable word-wrap for code blocks.

For touch devices, we could already disable word-wrap today (see my previous comment), as it would result in a behavior that is consistent with overflowing tables (they scroll on mobile, but - I'm fairly certain - not on desktop).

@OnkarRuikar Is there any CSS property that adds a symbol when the browser wraps the text? Would that character be shown on the previous or next line?

@schalkneethling
Copy link
Contributor

I reckon this is a valuable discussion to have. I propose that we do the following:

Let’s open a discussion on mn-community on how linting can help here, what other options we have for the immediate future, and how we see listing being integrated.

For this pull request, @lex111, if you could add the suggestion by Claas, then we can merge this one and make progress toward a better user experience.

Thank you, everyone!

@Rumyra
Copy link
Contributor

Rumyra commented May 23, 2022

Let’s open a discussion on mn-community on how linting can help here

Just fyi @schalkneethling there's a few things scattered about (issues & discussions) on all types of linting. Might be worth trying to collate a few

@OnkarRuikar
Copy link
Contributor

@lex111 @Rumyra, if we provide line numbers to the code blocks, will it improve the reading experience?
img


Regarding linting, the line length rule md013 is disabled on mdn/content. If we turn it on, it'll flag everything not just code blocks. Also, the linter doesn't fix line lengths automatically, it just flags them. It'll have to be done manually. :(


@OnkarRuikar Is there any CSS property that adds a symbol when the browser wraps the text?

Unfortunately there is no pseudo-class to indicate that the element has wrapped. I tried to find a simple solution and ended up
adding lines to the code section. With this in place we can add the wrapping markers from server side, where Prismjs highlights the code.

@schalkneethling
Copy link
Contributor

Hi everyone, as mentioned in #6083 (comment) I believe a combination of what @lex111 initially proposed and the suggestion by @caugner moves us in the right direction. I started the following discussion to discuss this topic further and more broadly.
mdn/mdn-community#111

@schalkneethling
Copy link
Contributor

Hi there everyone,

We discussed how we display code examples on MDN Web Docs, and for the moment, we are going to move all of this to a discussion for the following reasons:

  1. Different people have different ideas and suggestions for improving the code examples on MDN Web Docs, but we need to reach a consensus before starting to implement any of this. There are many aspects to consider, such as the complexity it might add to the codebase, additional dependencies, and potentially an increased maintenance burden.
  2. We can justify an increase in complexity or even maintenance if we are sure that the change(s) we make has a positive UX impact for all users across devices.
  3. We also need to ensure that changes do not complicate the authoring experience
  4. Keep accessibility and performance in mind.

Keeping all of this in mind, we thought it best to close this pull request and #6372 and continue the discussion between staff and external staff contributors and the wider community to find a solution that benefits everyone.

Thank you for opening this pull request which aided in getting this conversation started. We believe in the long term, this will lead to a better experience for everyone using MDN Web Docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍🤝‍🧑 community contributions by our wonderful community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants