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

Add space around code #223

Merged
merged 3 commits into from
Nov 22, 2021
Merged

Add space around code #223

merged 3 commits into from
Nov 22, 2021

Conversation

merkushin
Copy link
Member

Fixes #168 #131

Changes proposed in this Pull Request

  • Add some space around code

Testing instructions

  • Add a code block with an underscore on the last line (it might be a single line of code) to a post
  • Preview the post on Linux with Firefox (for me Ubuntu 20.04 and Firefox 90.0)

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

@merkushin merkushin added this to the 3.6.1 milestone Nov 16, 2021
@merkushin merkushin self-assigned this Nov 16, 2021
@renatho
Copy link
Contributor

renatho commented Nov 18, 2021

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
Copy link
Member Author

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:

  • remove left padding for line numbers;
  • remove left padding for code lines;
  • add margin-right for line numbers;
  • add padding for the whole .syntaxhighlighter.
    I think it is a more consistent way of 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
Copy link
Contributor

renatho commented Nov 19, 2021

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
Copy link
Member Author

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

Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

Cool! Looks good and works well!

@merkushin merkushin merged commit ae01588 into master Nov 22, 2021
@merkushin merkushin deleted the fix/space-around-code branch November 22, 2021 13:27
@merkushin merkushin mentioned this pull request Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Underscore character not visible in last line of code
2 participants