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

Implement Style Settings options for Obisidian 1.3.0+'s PDF tools #43

Open
mbeckrich opened this issue Jun 14, 2023 · 10 comments
Open

Implement Style Settings options for Obisidian 1.3.0+'s PDF tools #43

mbeckrich opened this issue Jun 14, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@mbeckrich
Copy link
Collaborator

User request in Discord: https://discord.com/channels/907385605422448742/1027435548165558303/1111347548364492873

Relevant tag from Minimal theme: https://github.com/kepano/obsidian-minimal/releases/tag/6.3.3

@mbeckrich mbeckrich added the enhancement New feature or request label Jun 14, 2023
@mbeckrich mbeckrich self-assigned this Jun 14, 2023
@true-Grand
Copy link

Any updates?

@mbeckrich
Copy link
Collaborator Author

Any updates?

I'm working on this as part of the rewrite on the development branch. It's not in there yet, but I will update this issue once it makes it in. :)

@corigne
Copy link

corigne commented Sep 6, 2024

Hey, curious but is there any update on this?
PDF rendering in dark mode with this theme is not great, particularly when there are highlights in the text.

Edit: I just saw the project status issue. I hope you're doing well! Sorry I didn't check it sooner. I was unaware you might be dealing with health issues. Wishing you good health.

@mbeckrich
Copy link
Collaborator Author

mbeckrich commented Sep 6, 2024 via email

mbeckrich added a commit that referenced this issue Sep 6, 2024
@mbeckrich
Copy link
Collaborator Author

I have gone ahead and implemented PDF-related features in Style Settings using some trimmed scss from Kepano's Minimal Theme (https://github.com/kepano/obsidian-minimal/blob/0b98cc8ec4576e148057ce6c0ff751ab4df1ece4/src/scss/content/pdf.scss). Three new toggles, all of which should be off by default, have been added:

  1. Removes borders around individual PDF pages, akin to his "seamless" setting
  2. Dark mode now inverts and mostly inherits the given flavor's background color (my PDFs have been book scans, so some jagged edges of the "wrong" color can still seep in using this method if the original background is not a solid color—this is probably part of what tripped me up in understanding the issue here, oops!)
  3. Light mode now blends the theme's background color into the PDF

Two examples can be seen below. I have pushed these changes already, but still open to any feedback/suggestions on this topic and will leave the issue open for a bit. Thanks to everyone for their patience!

Screenshot 2024-09-06 at 12 04 12 AM
Screenshot 2024-09-06 at 12 03 09 AM

@corigne
Copy link

corigne commented Sep 6, 2024

Appreciate the update! I will download and test.

@corigne
Copy link

corigne commented Sep 6, 2024

This is a really big improvement, thank you for taking the time to look at it again.
Unfortunately, it looks like there might still be some text highlight color/contrast issues in dark mode.

All screenshots below were taken with seamless mode on; the results are the same with seamless mode off.

Light Mode (Blend OFF)

image

Light Mode (Blend ON)

image

Dark Mode (Darken PDF Background OFF)

image

Dark Mode (Darken PDF Background ON)

image

@mbeckrich
Copy link
Collaborator Author

This is a really big improvement, thank you for taking the time to look at it again. Unfortunately, it looks like there might still be some text highlight color/contrast issues in dark mode.

All screenshots below were taken with seamless mode on; the results are the same with seamless mode off.

Thank you for testing this for me! I am now testing this out on some pdfs of my own. Does this issue occur for you in Minimal as well? Feel free to send me one of your pdfs and I can try to design a fix around it, if you're comfortable with that. I've been playing around with all this for the last hour or so and not sure what the right solution is yet. Bit of brain fog here, so bear with me:

I think the issue is that the css works by inverting the background color mathematically, so when a pdf with an already dark background is put into dark mode Obsidian with the darken pdf toggle on, it inverts the dark background and makes it light. So, I have at the very least poorly named the toggle... That said, what you're reporting still seems present at least somewhat in Minimal using my own pdfs (currently, Minimal is not showing me inverted colors when viewing pdfs embedded in a doc, but will invert them if I choose to open them in a new window—assuming that is intentional).

Out of curiosity, have you tried the "Adapt to theme" feature before? Below is a pdf export of dark mode help.obsidian.md with a few highlights from Preview in it. When it's toggled off, it shows the pdf as I would expect (dark in dark mode Obsidian). And when I turn it on, it does indeed invert-ish the colors. That said, it doesn't do anything for some pdfs on both my Mac mini and book, which I assume is intentional, but in easy cases it seems to correctly invert the colors regardless of Obsidian's light/dark mode. If you can get it to work on your pdfs, it might be the most reliable way to get the functionality you want, as it seems like a built-in version of the Style Settings approach? This way you might not have the problem of a blanket application of the inversion to all your pdfs. I will keep poking at this! I am now realizing all this might be part of what made me put this off for so long.

image image

@corigne
Copy link

corigne commented Sep 7, 2024

I'm happy to continue to help troubleshoot.
So, it looks like "Adapt to Theme" was already toggled on. I'm not sure if it's on by default in Obsidian.
It looks like it was applying effects the highlighted text that were conflicting with the effects from the PDF styling of the theme.
Here's what the same PDF looks like with it toggled off, for reference.

If you still need a copy for testing, I can attach it here, it looks like the problem, in my case, might be a conflict with this existing feature.

Also, I appreciate your work on this greatly. I have some familiarity with CSS myself, so if there's anything I can do to help directly, I'll try to take a look at the code.

image

@mbeckrich
Copy link
Collaborator Author

I'm happy to continue to help troubleshoot.

No problem! I am happy to work on this and appreciate your patience. It's just been a while since I have looked at all this, so I am refreshing my memory as we go. If you could attach a pdf, that would be great. If you want to look at the scss from Minimal, that's here: https://github.com/kepano/obsidian-minimal/blob/master/src/scss/content/pdf.scss If you don't have scss setup on your machine or haven't used it before, you can use something like this as a pure css snippet. The most relevant parts are at .ctp-pdf-darken .workspace-leaf-content[data-type=pdf] and .ctp-pdf-blend .workspace-leaf-content[data-type=pdf]. When enabled and using Minimal, no inversion is applied when looking at an embedded pdf, however, it does style pdfs split with "Open in new window." When enabled and using Catppuccin, inversion is applied to both embedded pdfs and new window pdfs, so I think Minimal must have something else going on I haven't looked into yet!

body {
	--pdf-dark-opacity: 1;
}

.theme-light:not(.pdf-shadows-on),
.theme-dark:not(.pdf-shadows-on) {
	--pdf-shadow: none;
	--pdf-thumbnail-shadow: none;

	.pdf-viewer .page {
		border: 0;
	}
	.pdf-sidebar-container .thumbnailSelectionRing {
		padding: 0;
	}
	.pdf-sidebar-container .thumbnail::after {
		right: var(--size-4-2);
		bottom: var(--size-4-2);
	}
}

.theme-dark {
	--pdf-thumbnail-shadow: 0 0 1px 0 rgba(0,0,0,0.6);
	--pdf-shadow: 0 0 1px 0 rgba(0,0,0,0.6);

	.pdf-viewer .canvasWrapper {
		opacity: var(--pdf-dark-opacity);
	}
	.ctp-pdf-darken .workspace-leaf-content[data-type=pdf] {
		.pdf-viewer .canvasWrapper {
			filter: invert(1) hue-rotate(180deg);
			mix-blend-mode: screen;
		}
	}
}
.theme-light {
	.ctp-pdf-blend .workspace-leaf-content[data-type=pdf] {
		.pdf-viewer .canvasWrapper {
			mix-blend-mode: multiply;
		}
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants