Fix Indentation of Code Block in several pages#4805
Fix Indentation of Code Block in several pages#4805sarah11918 merged 11 commits intowithastro:mainfrom
Conversation
✅ Deploy Preview for astro-docs-2 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Genteure
left a comment
There was a problem hiding this comment.
I think indentation should be added to the code block under 3., instead of removing the indentation before the code block in 2..
The code block should be a child element of the ordered list.
Source text:
1. one
2. two
```js
console.log(1)
```
3. three
```js
console.log(1)
```Rendered result:
- one
- two
console.log(2)
- three
console.log(3)
The html looks like:
<ol>
<li>one</li>
<li>two <code>console.log(2)</code></li>
<li>three <code>console.log(3)</code></li>
</ol>Compared to:
1. one
2. two
```js
console.log(1)
```
3. three
```js
console.log(1)
```Rendered result:
- one
- two
console.log(2)- three
console.log(3)The html looks like:
<ol>
<li>one</li>
<li>two</li>
</ol>
<code>console.log(2)</code>
<ol>
<li>three</li>
</ol>
<code>console.log(3)</code>
My personal opinion is that this approach is inconsistent with the overall aesthetic of the pages. Overall, it varies from page to page in the documentation. I am just aiming for consistency in terms of the left margin. However, I am more than willing to make the changes if some more people agree! |
|
Hey Hey! I'm here to confirm that @Genteure is correct with what we intend to use in docs! Extra indentation in these numbered lists! It's a pain and we don't always get it right, but when we notice, we always do correct this way. So if you're happy to re-indent everything, @VoxelMC , that would be fabulous! 🙌 ➡️ |
Yes! I can definitely do that :) Watch out, inconsistent indentation! |
|
@VoxelMC are you still interested in saving us from our indentation?? 😄 |
Yup! I will get started on that soon! Just had a project deadline yesterday... :p |
I have modified the indentation of code blocks and tips for 19 pages, with some extra changes to ensure some consistency and readability.
|
@sarah11918 I believe I have found all of the culprits! There is one on the Cloudflare README, so I may open up a PR there too, for consistency's sake. Let me know if it's okay! |
|
Hi @VoxelMC ! Just to let you know, in future I would really appreciate if these were separate PRs. This is 18 files to go through and check previews, some with multiple changes at different points on a page, and it is now much bigger than the original PR. I appreciate the enthusiasm, and obviously we do want these all fixed! Just know that these changes turn a very simple PR, that can be merged quite quickly, into one that requires a lot of checking on the part of the reviewers. For example, this is not something I can approve in 30 seconds anymore when I have a spare minute. Now, I have to keep a checklist to make sure I haven't missed verifying any of the changes! I will edit this one as is, but in future please do not do this as it is not actually as helpful as you might think it is. 😅 A handy rule is, if your changes no longer reflect the original PR title, you may have gotten a little too enthusiastic in your changes... |
Yes of course, I am sorry! I realize my overstepping. I won't do it again, and from now on I will keep the perspective of the reviewers at the top of my list. I know you have a lot of work to do. |
sarah11918
left a comment
There was a problem hiding this comment.
Some comments going through below! I appreciate your enthusiasm, but as you'll see from some of my comments, this really went off the rails and it can't happen again. 😄
It's not just that it took me more than 30 minutes to review (instead of 30 seconds), our translators also use these PRs as the source to update language files. Now, it becomes difficult for them to update because so many changes happened at once.
Thanks for understanding and working with our process that tries to make things as straightforward as possible for everyone! 🙌
| - npm run build | ||
| serve: dist | ||
| ``` | ||
| ```yaml title="Spacefile" {6,8,9} |
There was a problem hiding this comment.
This does not render indented, probably needs another set of tabs?
There was a problem hiding this comment.
It was the indentation of the numbered list above it. If you agree with my idea, i will revert this and open a separate PR to fix it there instead.
There was a problem hiding this comment.
Ah, looking more closely, neither of these should be indented since the code description isn't part of the step, the list of things are all things to add to the code snippet. So, the current version is fine and I'll remove the change on this page.
| return next(); | ||
| }; | ||
| ``` | ||
| ```js title="src/middleware.js" |
There was a problem hiding this comment.
This is also not indented in my preview, and is literally the single thing this PR was supposed to fix?? 😅
There was a problem hiding this comment.
Fixed, and checked that it is actually indented. I guess i fixed the one I changed but didn't change the one I was supposed to...
Note:
I would like this PR to be considered for HacktoberfestWhat kind of changes does this PR include?
Description
Corrected indentation for each language on the the middleware documentation page.
This pull request has been recreated from #4583 without the changes to i18n pages.
To