allefeld / atom-pdfjs-viewer

Themed, fully featured PDF viewer for the Atom editor
MIT License
12 stars 3 forks source link

no page break indicators #16

Closed Anonym234 closed 3 years ago

Anonym234 commented 3 years ago

There's no indicator for a page break, like a line or spacing between different pages in a PDF. This does not prohibit any use of the PDF-viewer, but makes it confusing to look at.

(using a Mac)

allefeld commented 3 years ago

Please submit a screenshot.

asiloisad commented 3 years ago

the same here: image

atom 1.57.0 x64

allefeld commented 3 years ago

Could you try changing your Atom color theme, to see whether that affects this problem?

asiloisad commented 3 years ago

in last post i have use "One Light" theme. "One Dark" theme has the same problem.

image

allefeld commented 3 years ago

Thanks!

Next, please open the same pdf in the official PDF.js viewer: https://mozilla.github.io/pdf.js/web/viewer.html

There is an "Open File" button in the top right.

asiloisad commented 3 years ago

screenshot from web mozilla browser: image

asiloisad commented 3 years ago

i have delete .atom, run atom, install only pdfjs-viewer and open the same file. problem still occurs: image tested with One Light and One Dark.

allefeld commented 3 years ago

Thanks for checking!

The problem is, I have no idea why this would happen, how to fix it, or even how to debug it, because it doesn't happen for me.

Can you send me the (a) pdf file with which this happens?

asiloisad commented 3 years ago

the problem occur in all pdf files. the file from screenshots is: main.pdf

allefeld commented 3 years ago

This is what the file looks like for me: Screenshot_20210527_182055

@asiloisad, are you also on a Mac?

asiloisad commented 3 years ago

win 10 x64 20H2 (latest update). it is a pity we cannot solve this problem, but luckily -- this is not a critical issue.

Anonym234 commented 3 years ago

new insight:

The issue is not present with the UI Theme 'Atom Light'. With every other Theme (Atom Dark, One Light, One Dark) it is present.

maybe this helps to resolve the issue. But, as @asiloisad pointed out, it's a non-critical issue, only slightly disturbing from time to time.

vishrutmishra commented 3 years ago

I am having the same issue. Exactly as @Anonym234 said: changing to UI Theme 'Atom Light' fixes it, but for any other UI Theme, the issue is present.

asiloisad commented 3 years ago

the solution is change margin in pdfjs/web/viewer.less:348 to margin: 1px auto 8px auto; instead of margin: 1px auto -8px auto;. in most themes space between pages is very good, but in Atom Light is too big.

allefeld commented 3 years ago

@asiloisad, I didn't understand what the color theme should have to do with margin width, but following your comment I investigated a bit further.

It turns out that the problem also goes away if I remove an import of atom.less (in viewer.less), which I had there to make sure that the scrollbar follows the color theme, too. As a consequence, the scroll bar in the PDF viewer will no longer be exactly like elsewhere, but in the four standard themes it blends in reasonably well, and most importantly the space between pages is always there.

I'll release a new version, please check whether that fixes the issue for you without asiloisad's hack. The relevant commit is c654b0b6da090724a48f0b51feb0e330b41e48ea.

This hopefully also fixes #5 for good.

asiloisad commented 3 years ago

the new version solved the problem of missing page separator... and broke the design of the page scroll bar :) I wish we had managed to keep a compatible design bar to scroll through the pages.

allefeld commented 3 years ago

I have to admit that I still don't understand exactly how the problem occurs in the first place.

In the four standard UI themes, the style file I imported previously (atom.less) is: atom-dark-ui atom-light-ui one-dark-ui one-light-ui

There's mainly scrollbar-styling in there, which is why I imported it in the first place, and which shouldn't affect the space between pages in the pdf viewer.

My guess is the cause of the problem is this code:

* {
    box-sizing: border-box;
}

which is in all these files except atom-light-ui. I don't understand why a color theme would include CSS which changes box metrics all over the place. IMHO this is just another example that Atom is a pretty unfinished piece of software.

So, if we wanted to keep the scrollbar styling, we would need to find a way to exclude this code from the import, or undo it. Suggestions welcome!

However, there's also #5, where apparently on Mac the import of the scrollbar styling fails in general (???), so for the moment I'm relatively happy with the current state.

allefeld commented 3 years ago

Closing for now, but feel free to create a new issue about the scrollbar styling.