Automattic / syntaxhighlighter

WordPress plugin that makes it easy to post syntax-highlighted code snippets.
https://alex.blog/wordpress-plugins/syntaxhighlighter/
239 stars 78 forks source link

Add space around code #223

Closed merkushin closed 2 years ago

merkushin commented 2 years ago

Fixes #168 #131

Changes proposed in this Pull Request

Testing instructions

Screenshot / Video

Before:

CleanShot 2021-11-16 at 15 22 02@2x CleanShot 2021-11-16 at 15 22 49@2x

After:

CleanShot 2021-11-16 at 15 17 44@2x CleanShot 2021-11-16 at 15 17 23@2x
renatho commented 2 years ago

Hum. I think it's better for cases with a contrast color in the block and without line number (WP-admin > Settings > SyntaxHighlighter - Color Theme and Display line numbers settings).

Screen Shot 2021-11-18 at 12 48 13

But for the same color with line numbers (screenshots from the PR description), it seems very spaced.

Also, with contrast and line numbers, I think it seems a little weird:

Screen Shot 2021-11-18 at 12 50 28

Maybe we could add the space only when it doesn't have the line numbers. And for the other case, we could just add 1px in the top and bottom to fix the underline issue. WDYT?

merkushin commented 2 years ago

I think we can do even more straightforward: remove padding only on the left of the line.

CleanShot 2021-11-19 at 16 17 00@2x

Or do those spaces around the block look weird for you? 🤔

CleanShot 2021-11-19 at 16 23 02@2x

As an development of the idea of removing/adding padding:

However, it doesn't solve the problem with seemingly too large padding when the code and the page have the same background. But we can set padding to 0.5em for top&bottom:

CleanShot 2021-11-19 at 16 34 53@2x CleanShot 2021-11-19 at 16 35 44@2x

The main problem here (from my perspective) is that we can't rely only on the theme of SyntaxHighlighter :-/ Otherwise, we could set different padding for different themes.

Aw, and I think 1px would look not so good in your example (I mean dark background for code and light background for the rest of the page, or vice versa). Better than #131 but too close to edges from my point of view.

CleanShot 2021-11-19 at 16 41 54@2x

Unfortunately, 1px is not enough to recognize underscore. Here is how it looks with 2px:

CleanShot 2021-11-19 at 16 42 24@2x
renatho commented 2 years ago

The spacings you added now look good to me! I just noticed that with the new approach, the title is misaligned (we can add it through the settings too).

Screen Shot 2021-11-19 at 12 11 25

I think the less we can change there would be better because the users can have CSS overriding some styles. So if we change less, there are less chances to break site customizations. :)

merkushin commented 2 years ago

Removed padding from caption.

Chrome Mac OS:

CleanShot 2021-11-22 at 12 16 42@2x

Firefox @ Ubuntu with line numbers:

CleanShot 2021-11-22 at 15 52 22@2x

Firefox @ Ubuntu without line numbers:

CleanShot 2021-11-22 at 15 53 26@2x