nmrium icon indicating copy to clipboard operation
nmrium copied to clipboard

feat: print spectra

Open hamed-musallam opened this issue 2 years ago • 2 comments

  • #2781

hamed-musallam avatar Dec 04 '23 12:12 hamed-musallam

Deploying nmrium with  Cloudflare Pages  Cloudflare Pages

Latest commit: d408ec4
Status: ✅  Deploy successful!
Preview URL: https://c448f1bc.nmrium.pages.dev
Branch Preview URL: https://print-spectra.nmrium.pages.dev

View logs

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (840c3e5) 53.42% compared to head (48eaf2d) 53.42%.

:exclamation: Current head 48eaf2d differs from pull request most recent head 0073612. Consider uploading reports for the commit 0073612 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2786   +/-   ##
=======================================
  Coverage   53.42%   53.42%           
=======================================
  Files          52       52           
  Lines        2456     2456           
  Branches       95       95           
=======================================
  Hits         1312     1312           
  Misses       1143     1143           
  Partials        1        1           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Dec 04 '23 12:12 codecov[bot]

@hamed-musallam

Could you finalize this PR ?

The goal is that when you print from the browser fill either an A4, B4, Letter (?) page depending of your general preferences.

It could be related to: https://stackoverflow.com/questions/16649943/css-to-set-a4-paper-size

Currently we have in the bottom the grey banner that should not be present and some green edit buttons on the top.

The size is also not setup automatically and it currently depends on the size of your browser window.

image

lpatiny avatar Jul 05 '24 11:07 lpatiny

We have currently 3 zones that should not be displayed

  • the outline of the molecule
  • 2 grey squares in the bottom
image

We after to find out how the NMRium div can be resized to either A4 or letter before printing.

Paper size (in mm) could be defined in the General Preferences using one of those options:

  • A4: 297 x 210
  • A3: 420 x 297
  • Letter: 279 x 216
  • Tabloid: 432 x 279

I guess we will need some padding depending the printers.

In CSS size can be specified in mm

lpatiny avatar Jul 09 '24 21:07 lpatiny

Google docs offers similar feature but in our case it will always be landscape: image

lpatiny avatar Jul 10 '24 08:07 lpatiny

Looks great, thanks !

Couple of little points:

I found out that the place we put the delta(ppm) is different in 1D and 2D.

1D it is in the grey banner in the bottom: image

2D it is over this grey banner image

This has as consequence than when printing currently we need to display this banner and for 2D it appears like an empty grey banner:

image

2 possibilities:

  • for 1D we move the d(ppm) over the grey bottom banner and this banner is hidden when printing
  • for 2D we move the d(ppm) in the grey zone and when we print the grey banner becomes white

I would go for the first possibility even if we loose a little bit of space for the spectrum.

The second issue is how the size of the paper is expected to be decided ? I didn't see in the code the selection of the paper size and if I change to A3 for example the spectrum would not enlarge:

2024-07-11 06 56 11

On firefox there are also 2 pages rather than one:

image

Did you know that you can emulate the @media: print from the developer tools? SHIFT + CMD + P and then look for media. You can then inspect the dom of the page as it would be rendered for priting.

image

When I do this, we have the panels on the right and a grey zone at the bottom:

image

If you need a new feature to hide the panels when printing (from react-science), do not hesitate to ask us.

lpatiny avatar Jul 11 '24 05:07 lpatiny

@targos Hamed wrote me:

I'm thinking, Instead of hiding the panels, toolbars, and header, and change the structure we can check whether it's possible to inject the viewer into an iframe and print from there. This needs to be investigated

You was speaking to me about some features of react that allows to copy dom possibly in another window. Could you detail a little bit this idea ?

lpatiny avatar Jul 11 '24 05:07 lpatiny

My idea is really abstract for now. I don't know how to do it (the React feature is createPortal)

targos avatar Jul 11 '24 12:07 targos

Would using https://stackoverflow.com/questions/1234008/detecting-browser-print-event help in order to catch the 'File -> print' event ?

lpatiny avatar Jul 16 '24 10:07 lpatiny

In seems also that when you cancel the print dialog box there is no resize and we stay with the zoomed layout.

2024-07-16 12 29 48

lpatiny avatar Jul 16 '24 10:07 lpatiny

Dialog box is really a great idea ! Could you save this information in the workspace as well so that it remembers the choice ? I don't think it is the case currently.

lpatiny avatar Jul 16 '24 11:07 lpatiny

it still not, We should save them within the workspace. Should we store them at the top level as follows?

{
  printPageOptions: {
    layout: 'portrait',
    size: 'A4',
    padding: '0cm'
  }
}

Or should they be included under the 'general' section in the 'General' tab of the general settings?

hamed-musallam avatar Jul 16 '24 11:07 hamed-musallam

As long as we always present the print dialog we don't need to add it in the general preferences but this could evolve in the future. It could become part of the general settings and be editable there as well.

lpatiny avatar Jul 16 '24 12:07 lpatiny

@lpatiny

can you test again on Firefox, Chrome, and Safari

hamed-musallam avatar Jul 18 '24 12:07 hamed-musallam