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

Fix inline code in dark mode when printing #1404

Merged
merged 6 commits into from
Dec 5, 2020

Conversation

raysonkoh
Copy link
Contributor

@raysonkoh raysonkoh commented Dec 5, 2020

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Fixes #1401

Overview of changes:
Updated the css selector in markbind.css, under the media print query for code blocks, to additionally target inline code.

Anything you'd like to highlight / discuss:

  • The accuracy of the CSS selector used to target inline code - code.hljs.inline
  • Color of inline-code might be too similar to the background color in <box>

image

Testing instructions: Ran npm run test, no errors.

Proposed commit message: (wrap lines at 72 characters): Fix inline code in dark mode when printing


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No blatantly unrelated changes
  • Pinged someone for a review!

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Dec 5, 2020

Thanks! 👍

The accuracy of the CSS selector used to target inline code - code.hljs.inline

looks good, I think this is as specific as it can get

Color of inline-code might be too similar to the background color in <box>

Untitled

Rather, it may look too much like normal text. Should we add a border similar to code blocks?

@damithc
Copy link
Contributor

damithc commented Dec 5, 2020

Rather, it may look too much like normal text. Should we add a border similar to code blocks?

If this refers to inline code, a border may be too visually jarring. How about a slightly shaded bg?
Example (from GitHub):

image

BTW, whichever it is, should be the same when using the light theme for the code, right?

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Dec 5, 2020

BTW, whichever it is, should be the same when using the light theme for the code, right?

Not necessarily; I think its ok to provide only a simpler, at-least printable dark theme.
Duplicating the light theme requires duplicating the styles which can be harder to maintain, and a more bloated bundle.

also when printing textbooks I really doubt many would print with colour anyway =P

@raysonkoh
Copy link
Contributor Author

raysonkoh commented Dec 5, 2020

I tried adding the same border as the code blocks, but with a smaller border-width size. Would this be okay?

image
Edit: lighter border-color

@damithc
Copy link
Contributor

damithc commented Dec 5, 2020

Not necessarily; I think its ok to provide only a simpler, at-least printable dark theme.
Duplicating the light theme requires duplicating the styles which can be harder to maintain, and a more bloated bundle.

I see. I was under the impression that we are switching to the light theme for printing but if that's not the case, a simple style fix is fine.

also when printing textbooks I really doubt many would print with colour anyway =P

Ink saving applies to B&W too :-)

@damithc
Copy link
Contributor

damithc commented Dec 5, 2020

I tried adding the same border as the code blocks, but with a smaller border-width size. Would this be okay?

image
Edit: lighter border-color

Looks OK to me. @ang-zeyu what do you think?

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Dec 5, 2020

Lgtm 👍 thanks! @raysonkoh

@ang-zeyu ang-zeyu added this to the v3.0 milestone Dec 5, 2020
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.

Code blocks: inconsistent light/dark mode use when printing
3 participants