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

Optimize code printing #2445

Merged

Conversation

yucheng11122017
Copy link
Contributor

@yucheng11122017 yucheng11122017 commented Mar 8, 2024

What is the purpose of this pull request?

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

Resolves #2438

Overview of changes:
Add CSS which

  • Makes code light theme automatically when printing
  • Removes code wrap and code copy buttons when printing
  • Include code highlighting when printing

Anything you'd like to highlight/discuss:
If anyone can help to test on Mac, that will be great.

There seems to be some issue with printing on Edge.. the CSS isnt applied on the preview even though the emulator reflects the changes..
TLDR: Don;t use Edge to print

Testing instructions:
Make code theme dark

function add(a, b) {
    const sum = a + b;
    console.log(`${a} + ${b} = ${sum}`);
    return sum;
}

Notice that code wrap and code copy buttons are gone
Code highlight is printed
Code theme is light

Proposed commit message: (wrap lines at 72 characters)
Optimize code blocks for printing


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.22%. Comparing base (371228d) to head (667502f).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2445      +/-   ##
==========================================
+ Coverage   49.04%   51.22%   +2.17%     
==========================================
  Files         124      124              
  Lines        5262     5281      +19     
  Branches     1118     1121       +3     
==========================================
+ Hits         2581     2705     +124     
+ Misses       2376     2290      -86     
+ Partials      305      286      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@itsyme
Copy link
Contributor

itsyme commented Mar 8, 2024

Tested it out on MacOS on Chrome and Arc browsers, the making code theme light automatically and the removal or buttons seems to work, but the code highlighting doesn't seem to work for me when printing.
Screenshot 2024-03-08 at 11 21 24 PM

Copy link
Contributor

@Tim-Siu Tim-Siu left a comment

Choose a reason for hiding this comment

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

I am also not able to observe code highlighting.

Another interesting problem is that although we automatically switched to the light theme in printing, the code highlighting color isn't updated and it is comparatively muted.

Above: light colour theme highlight scheme:
Screenshot 2024-03-09 at 12 46 39

Below: dark colour theme highlight scheme:
Screenshot 2024-03-09 at 12 47 54

Copy link
Contributor

Choose a reason for hiding this comment

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

It may be necessary to override the default code highlight color scheme for dark themes when printing. Otherwise, the color seems to be slightly muted.

@yucheng11122017
Copy link
Contributor Author

Hi @itsyme and @Tim-Siu thank you for the work on reviewing! I have made some changes accordingly. Do you mind helping me check on Mac again?

I'm not sure what's with the highlighting not appearing on the print preview despite it being on the emulator.. Could you help me check if thats the case on Safari and etc as well?

@Tim-Siu
Copy link
Contributor

Tim-Siu commented Mar 9, 2024

Hi @itsyme and @Tim-Siu thank you for the work on reviewing! I have made some changes accordingly. Do you mind helping me check on Mac again?

I'm not sure what's with the highlighting not appearing on the print preview despite it being on the emulator.. Could you help me check if thats the case on Safari and etc as well?

Hi @yucheng11122017 ,
The highlighting is still not working on Mac though.

@yucheng11122017
Copy link
Contributor Author

Hi @itsyme and @Tim-Siu thank you for the work on reviewing! I have made some changes accordingly. Do you mind helping me check on Mac again?
I'm not sure what's with the highlighting not appearing on the print preview despite it being on the emulator.. Could you help me check if thats the case on Safari and etc as well?

Hi @yucheng11122017 , The highlighting is still not working on Mac though.

On which browser? Could you check the emulator as well?

@Tim-Siu
Copy link
Contributor

Tim-Siu commented Mar 9, 2024

Hi @itsyme and @Tim-Siu thank you for the work on reviewing! I have made some changes accordingly. Do you mind helping me check on Mac again?
I'm not sure what's with the highlighting not appearing on the print preview despite it being on the emulator.. Could you help me check if thats the case on Safari and etc as well?

Hi @yucheng11122017 , The highlighting is still not working on Mac though.

On which browser? Could you check the emulator as well?

Hi @yucheng11122017

May I know what is the emulator? Do you mean the preview before printing?

The code highlighting isn't working on either Safari or Chrome in both preview and actual printed files.

@yucheng11122017
Copy link
Contributor Author

Hi @itsyme and @Tim-Siu thank you for the work on reviewing! I have made some changes accordingly. Do you mind helping me check on Mac again?
I'm not sure what's with the highlighting not appearing on the print preview despite it being on the emulator.. Could you help me check if thats the case on Safari and etc as well?

Hi @yucheng11122017 , The highlighting is still not working on Mac though.

On which browser? Could you check the emulator as well?

Hi @yucheng11122017

May I know what is the emulator? Do you mean the preview before printing?

The code highlighting isn't working on either Safari or Chrome in both preview and actual printed files.

You can try to open dev tools -> rendering -> print emulation.
https://stackoverflow.com/questions/9540990/using-chromes-element-inspector-in-print-preview-mode

@Tim-Siu
Copy link
Contributor

Tim-Siu commented Mar 9, 2024

Hi @itsyme and @Tim-Siu thank you for the work on reviewing! I have made some changes accordingly. Do you mind helping me check on Mac again?
I'm not sure what's with the highlighting not appearing on the print preview despite it being on the emulator.. Could you help me check if thats the case on Safari and etc as well?

Hi @yucheng11122017 , The highlighting is still not working on Mac though.

On which browser? Could you check the emulator as well?

Hi @yucheng11122017
May I know what is the emulator? Do you mean the preview before printing?
The code highlighting isn't working on either Safari or Chrome in both preview and actual printed files.

You can try to open dev tools -> rendering -> print emulation. https://stackoverflow.com/questions/9540990/using-chromes-element-inspector-in-print-preview-mode

Hi @yucheng11122017 ,
Screenshot 2024-03-09 at 17 57 21
This is what I get from the emulator. We do see highlight colour here.

@itsyme
Copy link
Contributor

itsyme commented Mar 10, 2024

I tried it on the emulator and it works for me as well however I'm not sure if it working on the emulator may be a reliable indicator of it working when printing in the physical form (https://stackoverflow.com/questions/26123622/print-preview-not-the-same-as-print-emulation-in-chrome) as when I downloaded to PDF (below) the highlights were still not appearing for me. To be 100% sure I can try printing a physical copy out to check if the highlights work.
Screenshot 2024-03-10 at 8 03 53 AM

@yucheng11122017
Copy link
Contributor Author

I tried it on the emulator and it works for me as well however I'm not sure if it working on the emulator may be a reliable indicator of it working when printing in the physical form (https://stackoverflow.com/questions/26123622/print-preview-not-the-same-as-print-emulation-in-chrome) as when I downloaded to PDF (below) the highlights were still not appearing for me. To be 100% sure I can try printing a physical copy out to check if the highlights work. Screenshot 2024-03-10 at 8 03 53 AM

I think its cool @itsyme. I tried printing out that day but it doesn't work as well.. Let me see if there's a workaround

@yucheng11122017 yucheng11122017 changed the title Fix print code highlight Optimize code printing Mar 10, 2024
@tlylt
Copy link
Contributor

tlylt commented Mar 11, 2024

I think its cool @itsyme. I tried printing out that day but it doesn't work as well.. Let me see if there's a workaround

I believe the generated pdf (print to pdf) would be the source of truth, if it has highlight, then the printed version will do too.

TLDR: Don;t use Edge to print

Different browsers might have different implementations hence affecting the outcome. We should highlight the recommended (and maybe the unsupported browsers) if we couldn't make it work for all.

@itsyme
Copy link
Contributor

itsyme commented Mar 13, 2024

Perhaps it would be good to include the findings in the user guide about which browsers are the most optimal for printing so that users interested in printing can refer to it!

@yucheng11122017
Copy link
Contributor Author

Sorry about the delay on this issue. I have asked @Tim-Siu to try to investigate this issue on Mac. I suspect that there might not be a workaround but he will update on what he has tried and research :)

@Tim-Siu
Copy link
Contributor

Tim-Siu commented Mar 15, 2024

HI @yucheng11122017 ,

I found this discussion to be relevant. It appears that adding -webkit-print-color-adjust:exact; in the element css will force print the background colour.

Example:

    .hljs span.highlighted{
        -webkit-print-color-adjust:exact;
        background:#e6e6fa !important;
    }

Output:

Screenshot 2024-03-15 at 20 05 57

Alternatively, users can also choose to print background colour themselves in Chrome settings under more settings -> options.
Screenshot 2024-03-15 at 20 04 10

You may refer to my branch which seems to have resolved the issue!

@kaixin-hc
Copy link
Contributor

might want to make sure it "works" on safari and firefox also - i believe they use the non-prefixed css (stack overflow link)

@kaixin-hc kaixin-hc added this to the v5.3.1 milestone Mar 15, 2024
@Tim-Siu
Copy link
Contributor

Tim-Siu commented Mar 18, 2024

might want to make sure it "works" on safari and firefox also - i believe they use the non-prefixed css (stack overflow link)

Hi @kaixin-hc ,

This approach works for Safari as well. (Maybe because Safari is also based on WebKit) It doesn't work for Firefox though. From my observations, there seem to have been many other bugs when printing with Firefox. I am not sure if it is necessary to fix them all.

Imag: printing result with firefox and current master branch
Screenshot 2024-03-18 at 12 09 27

@kaixin-hc
Copy link
Contributor

kaixin-hc commented Mar 18, 2024

Great investigation! @yucheng11122017 - perhaps you can implement the fix @Tim-Siu suggests, and then include a documentation note that there are known errors with printing on Edge and Firefox?

In terms of if we should fix it for Firefox/Edge, though it would be nice to figure out why and support it, if printing is OK on Chrome and Safari I think it is OK to deprioritise this (since a brief google search claims 70% of people are on Chrome or safari, with only 7% on Edge or Firefox)

Maybe can discuss in #2397 - probably related

@yucheng11122017
Copy link
Contributor Author

Apologies for the late response! I missed this in the slew of emails..
Thanks so much for the investigative work @Tim-Siu! It's really great and it seems to be working on Edge on Windows

image

I will push the fix and also add a caveat about Firefox in that case.

Copy link
Contributor

@itsyme itsyme left a comment

Choose a reason for hiding this comment

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

LGTM! Just a small nit in the wording of the documentation.

@yucheng11122017 yucheng11122017 merged commit 37b7756 into MarkBind:master Mar 22, 2024
9 checks passed
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 highlighting isn't visible for printing
5 participants