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

Recommend using semantic markup for code #35913

Closed
ojeytonwilliams opened this issue Apr 25, 2019 · 4 comments · Fixed by #35926
Closed

Recommend using semantic markup for code #35913

ojeytonwilliams opened this issue Apr 25, 2019 · 4 comments · Fixed by #35926
Labels
scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory.

Comments

@ojeytonwilliams
Copy link
Contributor

In the description and instruction sections of the challenges, the style guide recommends <blockquote> to markup blocks of code, and <code> for inline code.

I think it would be better to be consistent and use semantic markup for both.

We could either do this by through <pre><code> tags or take advantage of the fact we're using markdown and use code fences, i.e. ```js ```. This would have the benefit that we could use prismjs to make the code highlighting in the challenge descriptions the same as in the guide.

@thecodingaviator thecodingaviator added the status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. label Apr 25, 2019
@RandellDawson
Copy link
Member

I made the following changes to client/src/templates/Challenges/components/challenge-description.css

.challenge-instructions {
  font-size: 16px;
}

.challenge-instructions blockquote, .challenge-instructions pre {
  background-color: #eee;
  color: #c7254e;
  padding: 10px;
  width: 100%;
  margin: 0;
  margin-bottom: 1.45rem;
  font-family: monospace;
  font-size: 16px;
}

.challenge-instructions pre {
  border: 0;
  border-radius: 0%;
}

This allows the current way we are writing the blockquote code sections to look the same as long as the the code written immediately follows the <code> tag and the </code> tag immediately follows the written code.

For example, with the above CSS change, both of the following render exactly the same.

<blockquote>
{<br>
&nbsp;&nbsp;Alan: {<br>
&nbsp;&nbsp;&nbsp;&nbsp;online: false<br>
&nbsp;&nbsp;},<br>
&nbsp;&nbsp;Jeff: {<br>
&nbsp;&nbsp;&nbsp;&nbsp;online: true<br>
&nbsp;&nbsp;},<br>
&nbsp;&nbsp;Sarah: {<br>
&nbsp;&nbsp;&nbsp;&nbsp;online: false<br>
&nbsp;&nbsp;}<br>
}</blockquote>

<pre><code>{
  Alan: {
    online: false
  },
  Jeff: {
    online: true
  },
  Sarah: {
    online: false
  }
}</code></pre>

If the <code> tag` is on a separate line which would make it more readable (like below), then there would be an extra line space at the top (see very bottom screenshot).

<pre><code>
{
  Alan: {
    online: false
  },
  Jeff: {
    online: true
  },
  Sarah: {
    online: false
  }
}
</code></pre>

image

or take advantage of the fact we're using markdown and use code fences, i.e. js . This would have the benefit that we could use prismjs to make the code highlighting in the challenge descriptions the same as in the guide.

I really like the idea of being able to use the js markdown format, but if we could be OK with making sure to start the code immediately after the <pre><code>, that is a huge improvement over how we are having to write the blockquotes currently.

@ojeytonwilliams
Copy link
Contributor Author

I've had a quick play with your css and it works perfectly with fenced code blocks when no language isspecified. If you have the following

```
{
  Alan: {
    online: false
  },
  Jeff: {
    online: true
  }
}
```

it is converted to

<pre><code>{
  Alan: {
    online: false
  },
  Jeff: {
    online: true
  }
}
</code></pre>

As you can see, it puts the code immediately after the code tag, exactly as we wanted.

I used a blockquote, a pre code block and finally the above fenced code and got:
VariousCodeBlocks

Based on this, I think fenced code blocks are the way to go - regardless of how we end up styling them.

@RandellDawson
Copy link
Member

@ojeytonwilliams Just tested your use of the 3 backticks and works just like the pre/code version. I thought I tried something like that before, but it never formatted correctly. Oh well, at least you figured out we can possibly use this. The only thing we need to validate is that night mode will still render the colors and background as intended, because the only other place which styles blockquote elements from what I can tell is in client/src/components/layouts/night.css

.night .challenge-instructions blockquote,
.night blockquote,
.night pre {
  background: #222;
}

.night code {
  background-color: #242424;
  color: #02a902;
}

How do we "sign in" when using npm run develop locally? I want to change to night mode under settings, to make sure it still looks correct. If this looks good too, then I say we create the PR and ask @raisedadead to take a look. If all goes well and we can merge that PR, then the nice thing is the old way using blockquote will still work and allow us to slowly convert over to using the 3 backtick method. We could start with all outstanding English curriculum PRs and make the changes there and then maybe figure out a programmatic solution which targets only the Description and Instructions sections and literally converts the &nbsp;s to spaces and <br>s to line breaks. Then we could just create individual PRs for each main section using a conversion script and "fixing" any of the edge cases that exist. If we were diligent, I think we could fix all the English challenges in 1-2 weeks max.,

@RandellDawson RandellDawson added scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. language: English labels Apr 26, 2019
@ojeytonwilliams
Copy link
Contributor Author

@RandellDawson It's been a while since I created my local account, but I believe you use npm run seed:auth-user. If it works you should be able to sign in at http://localhost:8000/signin and then access settings normally. If it doesn't work, you can replace the class default with night in the html element, as a temporary hack.

I like the idea of a smooth transition from blockquotes. Ideally we'd add the language labels as we go (but keep your style). That way, if we do decided to highlight the code later on, it should be a fairly painless change.

@RandellDawson RandellDawson added status: in progress and removed status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. labels May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory.
Projects
None yet
3 participants