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

Update markdown code display #16

Merged

Conversation

mfg92
Copy link
Contributor

@mfg92 mfg92 commented Jun 10, 2018

At the moment code is rendered like this (Windows, Firefox 61):
before

I think always showing the scrollbar doesn't look good. How about hiding it if it is not needed (first commit / d86ba2f):
after d86ba2f

Furthermore, I would also add a darker background to the code and make its width 100% of the content area (second commit / 706992a):
after 706992a

I have no background in design etc. so I am interested in your opinion @vickylai on these changes.

Test markdown:

Bla bla...:
```
test.code();
```

..bla bla.

@mfg92
Copy link
Contributor Author

mfg92 commented Jun 10, 2018

Now I noticed that code is rendered completely different if I specify a language for pygemnts in markdown.
So this is the main problem.
When pygemnts are used and the code is wrapped in a 'dive class="highlight"', always showing the scrollbar looks OK to me. But for my taste the width should be fixed at 100%.

victoriadrake pushed a commit that referenced this pull request Jun 10, 2018
+ Add code block background color as variable
+ Add 100% width to highlight class
@victoriadrake
Copy link
Owner

These changes are most definitely better!

Yes, the theme's default styles for pre will be overridden if these two lines are present in config.toml:

pygmentsStyle = "monokai" # Any theme name from: https://help.farbox.com/pygments.html
pygmentsCodefences = true

I agree about the 100% width.

@victoriadrake victoriadrake merged commit 706992a into victoriadrake:master Jun 10, 2018
@mfg92
Copy link
Contributor Author

mfg92 commented Jun 11, 2018

Thank you for your fast response.

I noticed a minor flaw in your changes in saas/_elements.sass:
Now there are two entries for .highlight. I think the lower one is not needed anymore.

victoriadrake pushed a commit that referenced this pull request Jun 11, 2018
+ As mentioned in pull request comment, highlight class was declared
twice.
@victoriadrake
Copy link
Owner

You're right. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants